Cleanup tracking #1886

martinthomson · 2024-05-07T05:24:37Z

A version of #1662 but retaining the BTreeMap backing store.

One thing I learned from this was that creating temporary stores was probably the biggest cost. It might be that the Vec could be better with that realization, but this has the advantage of being a little bit simpler.

Botched the "merge from main" on mozilla#1662. Reverted the merge. This PS has the changes in mozilla#1662, rebased to main. Sorry for the mess.

Signed-off-by: Lars Eggert <[email protected]>

neqo-transport/src/recovery/sent.rs

github-actions · 2024-05-07T05:39:32Z

Failed Interop Tests

QUIC Interop Runner, client vs. server

aioquic vs. neqo-latest: R A
go-x-net vs. neqo-latest: A
kwik vs. neqo-latest: A
lsquic vs. neqo-latest: R A
msquic vs. neqo-latest: Z A L1
mvfst vs. neqo-latest: Z 3 A L1 C1
neqo vs. neqo-latest: A C1
neqo-latest vs. aioquic: Z
neqo-latest vs. kwik: Z
neqo-latest vs. lsquic: Z
neqo-latest vs. msquic: Z A
neqo-latest vs. mvfst: DC B U A L2 C1 C2
neqo-latest vs. neqo: A
neqo-latest vs. neqo-latest: Z A C1
neqo-latest vs. ngtcp2: Z
neqo-latest vs. quiche: L1 C1
neqo-latest vs. quinn: E A
neqo-latest vs. s2n-quic: R
neqo-latest vs. xquic: Z A C1
ngtcp2 vs. neqo-latest: A
picoquic vs. neqo-latest: R A
quic-go vs. neqo-latest: A
quiche vs. neqo-latest: R 3 A
quinn vs. neqo-latest: R Z E A
s2n-quic vs. neqo-latest: A
xquic vs. neqo-latest: M A C1

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

aioquic vs. neqo-latest: H DC LR C20 M S Z 3 B L1 L2 C1 C2 6
chrome vs. neqo-latest: 3
go-x-net vs. neqo-latest: H DC LR M B U L2 C2 6
kwik vs. neqo-latest: H DC LR C20 M S R Z 3 B U L1 L2 C1 C2 6 V2
lsquic vs. neqo-latest: H DC LR M S 3 B E L1 L2 C1 C2 6 V2
msquic vs. neqo-latest: H DC LR C20 M S R B U L2 C1 C2 6 V2
mvfst vs. neqo-latest: H DC LR M B L2 C2 6
neqo vs. neqo-latest: H DC LR C20 M S R Z 3 B U E L1 L2 C2 6 V2
neqo-latest vs. aioquic: H DC LR C20 M S R 3 B U A L1 L2 C1 C2 6
neqo-latest vs. go-x-net: H DC LR M B U A L2 C2 6
neqo-latest vs. haproxy: H DC LR C20 M S R Z 3 B U A L1 L2 C1 C2 6 V2
neqo-latest vs. kwik: H DC LR C20 M S R 3 B U A L1 L2 C1 C2 6 V2
neqo-latest vs. lsquic: H DC LR C20 M S R 3 B U E A L1 L2 C1 C2 6 V2
neqo-latest vs. msquic: H DC LR C20 M S R B U L1 L2 C1 C2 6 V2
neqo-latest vs. mvfst: H LR M R Z 3 L1 6
neqo-latest vs. neqo: H DC LR C20 M S R Z 3 B U E L1 L2 C1 C2 6 V2
neqo-latest vs. neqo-latest: H DC LR C20 M S R 3 B U E L1 L2 C2 6 V2
neqo-latest vs. nginx: H DC LR C20 M S R Z 3 B U A L1 L2 C1 C2 6
neqo-latest vs. ngtcp2: H DC LR C20 M S R 3 B U E A L1 L2 C1 C2 6 V2
neqo-latest vs. picoquic: H DC LR C20 M S R Z 3 B U E A L1 L2 C1 C2 6 V2
neqo-latest vs. quic-go: H DC LR C20 M S R Z 3 B U A L1 L2 C1 C2 6
neqo-latest vs. quiche: H DC LR C20 M S R Z 3 B U A L2 C2 6
neqo-latest vs. quinn: H DC LR C20 M S R Z 3 B U L2 C2 6
neqo-latest vs. s2n-quic: H DC LR C20 M S 3 B U E A L1 L2 C1 C2 6
neqo-latest vs. xquic: H DC LR C20 M R 3 B U L1 L2 C2 6
ngtcp2 vs. neqo-latest: H DC LR C20 M S R Z 3 B U E L1 L2 C1 C2 6 V2
picoquic vs. neqo-latest: H DC LR C20 M S Z 3 B U E L1 L2 C1 C2 6 V2
quic-go vs. neqo-latest: H DC LR C20 M S R Z 3 B U L1 L2 C1 C2 6
quiche vs. neqo-latest: H DC LR M S Z B L1 L2 C1 C2 6
quinn vs. neqo-latest: H DC LR C20 M S 3 B U L2 C2 6
s2n-quic vs. neqo-latest: H DC LR M S R 3 B E L1 L2 C1 C2 6
xquic vs. neqo-latest: H DC LR C20 S R Z 3 B U L1 L2 C2 6

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

aioquic vs. neqo-latest: U E V2
chrome vs. neqo-latest: H DC LR C20 M S R Z B U E A L1 L2 C1 C2 6 V2
go-x-net vs. neqo-latest: C20 S R Z 3 E L1 C1 V2
kwik vs. neqo-latest: E
lsquic vs. neqo-latest: C20 Z U
msquic vs. neqo-latest: 3 E
mvfst vs. neqo-latest: C20 S R U E V2
neqo-latest vs. aioquic: E V2
neqo-latest vs. go-x-net: C20 S R Z 3 E L1 C1 V2
neqo-latest vs. haproxy: E
neqo-latest vs. kwik: E
neqo-latest vs. msquic: 3 E
neqo-latest vs. mvfst: C20 S E V2
neqo-latest vs. nginx: E V2
neqo-latest vs. quic-go: E V2
neqo-latest vs. quiche: E V2
neqo-latest vs. quinn: L1 C1 V2
neqo-latest vs. s2n-quic: Z V2
neqo-latest vs. xquic: S E V2
quic-go vs. neqo-latest: E V2
quiche vs. neqo-latest: C20 U E V2
quinn vs. neqo-latest: L1 C1 V2
s2n-quic vs. neqo-latest: C20 Z U V2
xquic vs. neqo-latest: E V2

github-actions · 2024-05-07T05:44:49Z

Firefox builds for this PR

The following builds are available for testing. Crossed-out builds did not succeed.

Linux: ~~Debug~~ Release
macOS: Debug Release
Windows: Debug Release

neqo-transport/src/cc/classic_cc.rs

neqo-transport/src/connection/mod.rs

github-actions · 2024-05-07T11:26:56Z

Benchmark results

Performance differences relative to 6979f01.

drain a timer quickly time: [308.97 ns 316.88 ns 324.52 ns]
change: [-2.3263% +0.1151% +2.7577%] (p = 0.93 > 0.05)
No change in performance detected.
coalesce_acked_from_zero 1+1 entries
time: [195.67 ns 196.09 ns 196.54 ns]
change: [-0.0442% +0.3370% +0.7130%] (p = 0.09 > 0.05)
No change in performance detected.
coalesce_acked_from_zero 3+1 entries
time: [237.49 ns 238.14 ns 238.81 ns]
change: [-0.0388% +0.3442% +0.7167%] (p = 0.08 > 0.05)
No change in performance detected.
coalesce_acked_from_zero 10+1 entries
time: [237.26 ns 238.28 ns 239.38 ns]
change: [-0.2810% +0.4419% +1.2845%] (p = 0.28 > 0.05)
No change in performance detected.
coalesce_acked_from_zero 1000+1 entries
time: [217.08 ns 217.31 ns 217.61 ns]
change: [-1.9746% -1.2613% -0.6121%] (p = 0.00 < 0.05)
Change within noise threshold.
RxStreamOrderer::inbound_frame()
time: [119.14 ms 119.20 ms 119.26 ms]
change: [+0.4638% +0.5578% +0.6497%] (p = 0.00 < 0.05)
Change within noise threshold.
transfer/Run multiple transfers with varying seeds
time: [118.84 ms 119.12 ms 119.39 ms]
thrpt: [33.504 MiB/s 33.581 MiB/s 33.658 MiB/s]
change:
time: [-0.3157% -0.0189% +0.2992%] (p = 0.91 > 0.05)
thrpt: [-0.2983% +0.0189% +0.3167%]
No change in performance detected.
transfer/Run multiple transfers with the same seed
time: [119.44 ms 119.67 ms 119.90 ms]
thrpt: [33.360 MiB/s 33.424 MiB/s 33.488 MiB/s]
change:
time: [+0.5597% +0.8290% +1.1277%] (p = 0.00 < 0.05)
thrpt: [-1.1152% -0.8222% -0.5566%]
Change within noise threshold.
1-conn/1-100mb-resp (aka. Download)/client
time: [1.1227 s 1.1257 s 1.1284 s]
thrpt: [88.623 MiB/s 88.835 MiB/s 89.069 MiB/s]
change:
time: [-1.8666% +0.7207% +3.0539%] (p = 0.60 > 0.05)
thrpt: [-2.9634% -0.7155% +1.9021%]
No change in performance detected.
1-conn/10_000-parallel-1b-resp (aka. RPS)/client
time: [428.28 ms 430.19 ms 432.15 ms]
thrpt: [23.140 Kelem/s 23.246 Kelem/s 23.349 Kelem/s]
change:
time: [+0.3817% +1.0505% +1.7371%] (p = 0.00 < 0.05)
thrpt: [-1.7074% -1.0396% -0.3803%]
Change within noise threshold.
1-conn/1-1b-resp (aka. HPS)/client
time: [49.713 ms 49.824 ms 49.961 ms]
thrpt: [20.016 elem/s 20.071 elem/s 20.115 elem/s]
change:
time: [-0.6318% -0.0762% +0.4928%] (p = 0.79 > 0.05)
thrpt: [-0.4904% +0.0762% +0.6358%]
No change in performance detected.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client	Server	CC	Pacing	Mean [ms]	Min [ms]	Max [ms]	Relative
msquic	msquic			433.5 ± 61.2	376.9	568.4	1.00
neqo	msquic	reno	on	792.5 ± 28.4	764.4	838.8	1.00
neqo	msquic	reno		866.4 ± 220.2	755.8	1455.2	1.00
neqo	msquic	cubic	on	791.4 ± 32.0	744.0	841.0	1.00
neqo	msquic	cubic		804.7 ± 85.0	746.1	1042.4	1.00
msquic	neqo	reno	on	5278.2 ± 219.9	4874.0	5558.8	1.00
msquic	neqo	reno		5270.6 ± 175.7	5041.4	5506.4	1.00
msquic	neqo	cubic	on	5059.2 ± 179.4	4824.2	5378.5	1.00
msquic	neqo	cubic		5044.6 ± 172.7	4819.8	5295.4	1.00
neqo	neqo	reno	on	4099.5 ± 347.9	3305.5	4440.1	1.00
neqo	neqo	reno		3883.1 ± 395.5	3002.3	4277.1	1.00
neqo	neqo	cubic	on	4747.0 ± 280.0	4163.8	5085.7	1.00
neqo	neqo	cubic		4665.5 ± 419.7	3646.0	5023.0	1.00

⬇️ Download logs

Signed-off-by: Lars Eggert <[email protected]>

codecov · 2024-05-07T12:11:49Z

Codecov Report

Attention: Patch coverage is 95.58360% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 93.20%. Comparing base (6979f01) to head (f26739d).

Files	Patch %	Lines
neqo-transport/src/cc/classic_cc.rs	77.77%	6 Missing ⚠️
neqo-transport/src/recovery/mod.rs	92.98%	4 Missing ⚠️
neqo-transport/src/recovery/sent.rs	98.62%	3 Missing ⚠️
neqo-transport/src/path.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1886      +/-   ##
==========================================
- Coverage   93.21%   93.20%   -0.02%     
==========================================
  Files         110      111       +1     
  Lines       35879    36027     +148     
==========================================
+ Hits        33446    33580     +134     
- Misses       2433     2447      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

neqo-transport/src/connection/mod.rs

neqo-transport/src/recovery/sent.rs

Signed-off-by: Lars Eggert <[email protected]>

larseggert and others added 12 commits March 19, 2024 16:27

fix: Resurrect mozilla#1662

b663879

Botched the "merge from main" on mozilla#1662. Reverted the merge. This PS has the changes in mozilla#1662, rebased to main. Sorry for the mess.

Merge branch 'main' into redo-1662

177adc6

Signed-off-by: Lars Eggert <[email protected]>

Fix

707a169

Merge branch 'main' into redo-1662

96104f1

Merge remote-tracking branch 'origin/main' into redo-1662

ff188da

One-liner

e538170

Merge main

5c0125a

Merge main

941ec91

Merge branch 'main' into cleanup-tracking

b7c3f30

Move back to BTreeMap

602930a

Tweak ordering

623dae1

More simplification/performance

3d0363b

martinthomson requested review from KershawChang and larseggert as code owners May 7, 2024 05:24

martinthomson commented May 7, 2024

View reviewed changes

neqo-transport/src/recovery/sent.rs Show resolved Hide resolved

martinthomson commented May 7, 2024

View reviewed changes

neqo-transport/src/recovery/sent.rs Show resolved Hide resolved

Test expectation fixup

4e175b4

larseggert reviewed May 7, 2024

View reviewed changes

neqo-transport/src/cc/classic_cc.rs Outdated Show resolved Hide resolved

neqo-transport/src/connection/mod.rs Show resolved Hide resolved

Merge branch 'main' into cleanup-tracking

c243e92

Update neqo-transport/src/cc/classic_cc.rs

f26739d

Signed-off-by: Lars Eggert <[email protected]>

larseggert enabled auto-merge May 7, 2024 13:41

larseggert approved these changes May 7, 2024

View reviewed changes

neqo-transport/src/connection/mod.rs Outdated Show resolved Hide resolved

neqo-transport/src/recovery/sent.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

22d57ce

Signed-off-by: Lars Eggert <[email protected]>

larseggert added this pull request to the merge queue May 7, 2024

Merged via the queue into mozilla:main with commit beffcc6 May 7, 2024
52 of 53 checks passed

martinthomson deleted the cleanup-tracking branch May 13, 2024 00:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup tracking #1886

Cleanup tracking #1886

martinthomson commented May 7, 2024

github-actions bot commented May 7, 2024 •

edited

Loading

Succeeded Interop Tests

Unsupported Interop Tests

github-actions bot commented May 7, 2024 •

edited

Loading

github-actions bot commented May 7, 2024 •

edited

Loading

codecov bot commented May 7, 2024

Cleanup tracking #1886

Cleanup tracking #1886

Conversation

martinthomson commented May 7, 2024

github-actions bot commented May 7, 2024 • edited Loading

Failed Interop Tests

Succeeded Interop Tests

Unsupported Interop Tests

github-actions bot commented May 7, 2024 • edited Loading

Firefox builds for this PR

github-actions bot commented May 7, 2024 • edited Loading

Benchmark results

Client/server transfer results

codecov bot commented May 7, 2024

Codecov Report

github-actions bot commented May 7, 2024 •

edited

Loading

github-actions bot commented May 7, 2024 •

edited

Loading

github-actions bot commented May 7, 2024 •

edited

Loading