fix: Resurrect #1662

[larseggert]

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

[codecov]

Codecov Report

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

Project coverage is 92.97%. Comparing base (7028479) to head (b663879).

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1761      +/-   ##
==========================================
- Coverage   92.97%   92.97%   -0.01%     
==========================================
  Files         119      121       +2     
  Lines       37253    37449     +196     
==========================================
+ Hits        34637    34819     +182     
- Misses       2616     2630      +14     

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

[github-actions]

Benchmark results

Performance differences relative to 2463618.

• drain a timer quickly time: [430.82 ns 437.38 ns 443.44 ns]
  change: [-0.4630% +1.2459% +3.2320%] (p = 0.22 > 0.05)
  No change in performance detected.

• coalesce_acked_from_zero 1+1 entries
  time: [193.78 ns 194.29 ns 194.84 ns]
  change: [-1.4754% -1.0282% -0.5436%] (p = 0.00 < 0.05)
  Change within noise threshold.

• coalesce_acked_from_zero 3+1 entries
  time: [237.85 ns 238.48 ns 239.15 ns]
  change: [-0.6616% -0.3631% -0.0819%] (p = 0.02 < 0.05)
  Change within noise threshold.

• coalesce_acked_from_zero 10+1 entries
  time: [237.07 ns 241.15 ns 249.35 ns]
  change: [-0.9774% +0.3127% +2.2973%] (p = 0.80 > 0.05)
  No change in performance detected.

• coalesce_acked_from_zero 1000+1 entries
  time: [219.16 ns 219.40 ns 219.70 ns]
  change: [-22.717% -9.4642% +0.1369%] (p = 0.22 > 0.05)
  No change in performance detected.

• RxStreamOrderer::inbound_frame()
  time: [118.25 ms 118.44 ms 118.68 ms]
  change: [-0.7960% -0.5398% -0.3065%] (p = 0.00 < 0.05)
  Change within noise threshold.

• transfer/Run multiple transfers with varying seeds
  time: [132.71 ms 133.01 ms 133.31 ms]
  thrpt: [30.005 MiB/s 30.073 MiB/s 30.140 MiB/s]
  change:
  time: [+10.985% +11.335% +11.695%] (p = 0.00 < 0.05)
  thrpt: [-10.470% -10.181% -9.8973%]
  💔 Performance has regressed.

• transfer/Run multiple transfers with the same seed
  time: [133.29 ms 133.49 ms 133.69 ms]
  thrpt: [29.920 MiB/s 29.965 MiB/s 30.009 MiB/s]
  change:
  time: [+10.250% +10.494% +10.736%] (p = 0.00 < 0.05)
  thrpt: [-9.6954% -9.4971% -9.2972%]
  💔 Performance has regressed.

• 1-conn/1-100mb-resp (aka. Download)/client
  time: [1.2634 s 1.2777 s 1.2912 s]
  thrpt: [77.445 MiB/s 78.263 MiB/s 79.154 MiB/s]
  change:
  time: [+9.8908% +11.640% +13.283%] (p = 0.00 < 0.05)
  thrpt: [-11.725% -10.426% -9.0006%]
  💔 Performance has regressed.

• 1-conn/10_000-parallel-1b-resp (aka. RPS)/client
  time: [422.65 ms 424.65 ms 426.68 ms]
  thrpt: [23.437 Kelem/s 23.549 Kelem/s 23.660 Kelem/s]
  change:
  time: [-2.0914% -1.4280% -0.7687%] (p = 0.00 < 0.05)
  thrpt: [+0.7746% +1.4487% +2.1361%]
  Change within noise threshold.

• 1-conn/1-1b-resp (aka. HPS)/client
  time: [49.958 ms 50.353 ms 50.741 ms]
  thrpt: [19.708 elem/s 19.860 elem/s 20.017 elem/s]
  change:
  time: [-1.9459% -0.7999% +0.1881%] (p = 0.14 > 0.05)
  thrpt: [-0.1877% +0.8064% +1.9845%]
  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			728.9 ± 269.5	408.0	1253.8	1.00
neqo	msquic	reno	on	1036.5 ± 226.5	781.9	1372.9	1.00
neqo	msquic	reno		916.9 ± 198.8	751.1	1382.3	1.00
neqo	msquic	cubic	on	1030.2 ± 248.6	794.9	1400.6	1.00
neqo	msquic	cubic		966.9 ± 227.2	738.3	1476.0	1.00
msquic	neqo	reno	on	5116.8 ± 195.5	4849.8	5360.3	1.00
msquic	neqo	reno		5222.1 ± 232.8	4935.3	5636.3	1.00
msquic	neqo	cubic	on	5346.2 ± 331.8	4943.1	6062.1	1.00
msquic	neqo	cubic		5139.3 ± 212.9	4765.8	5423.1	1.00
neqo	neqo	reno	on	4894.2 ± 481.0	3837.0	5626.1	1.00
neqo	neqo	reno		4952.5 ± 335.3	4273.9	5411.2	1.00
neqo	neqo	cubic	on	5037.5 ± 257.5	4706.1	5631.6	1.00
neqo	neqo	cubic		4955.9 ± 173.0	4702.4	5290.4	1.00

⬇️ Download logs

[larseggert]

@martinthomson at least you should have better flamegraphs now. Still looks slower.

[larseggert]

@martinthomson should we refactor this and pull the cleanups into the main branch?

[larseggert]

@martinthomson any chance you can extract the cleanups out of this one, so we can merge them? They keep rotting otherwise. (Or let me know if I should try.)

[larseggert]

@martinthomson I rebased it onto the current main. Should hopefully make it easier to revert the change to VecDeque?

[martinthomson]

Thanks. I started to poke at this yesterday and ran into a crash bug in the NSS initialization tests. I'm out of time though. This will have to wait a week or two. I'll look after the rebasing for that.

[larseggert]

@martinthomson any chance we could land the cleanups soon-ish?

[martinthomson]

I've a new branch with fixes on it. Some seem to improve things, but we can check there.