Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the add_datagram method in the connection events handling to improve efficiency by performing a single iteration over the events queue instead of two separate passes. The change eliminates the check_datagram_queued helper function and consolidates its logic directly into add_datagram, combining the counting and head-drop logic into one loop.
Key changes:
- Merged
check_datagram_queuedfunction intoadd_datagramto eliminate redundant iteration - Implemented single-pass algorithm that tracks the first datagram index while counting remaining capacity
Comment on lines
+178
to
+180
| let Some(first_idx) = first_idx else { | ||
| unreachable!("first_idx is Some when datagrams_remaining is 0"); | ||
| }; |
There was a problem hiding this comment.
The unreachable! can never be reached because first_idx is guaranteed to be Some when datagrams_remaining == 0. The logic ensures we only reach this point after finding at least one datagram (which sets first_idx). Consider using unwrap() with a comment explaining the invariant, or restructure to avoid the pattern match entirely since you already know first_idx.is_some() at this point.
Suggested change
| let Some(first_idx) = first_idx else { | |
| unreachable!("first_idx is Some when datagrams_remaining is 0"); | |
| }; | |
| // Invariant: first_idx is always Some here because datagrams_remaining only reaches 0 after finding at least one datagram. | |
| let first_idx = first_idx.unwrap(); |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3239 +/- ##
==========================================
- Coverage 93.47% 93.43% -0.04%
==========================================
Files 125 125
Lines 36582 36578 -4
Branches 36582 36578 -4
==========================================
- Hits 34194 34177 -17
- Misses 1543 1556 +13
Partials 845 845
|
Contributor
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to 042e684. All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Contributor
Client/server transfer resultsPerformance differences relative to baf9d05. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
Contributor
Benchmark resultsSignificant performance differences relative to baf9d05. 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: 💔 Performance has regressed by +2.5159%. time: [204.41 ms 204.75 ms 205.16 ms]
thrpt: [487.43 MiB/s 488.40 MiB/s 489.21 MiB/s]
change:
time: [+2.2390% +2.5159% +2.8188] (p = 0.00 < 0.05)
thrpt: [-2.7415% -2.4541% -2.1900]
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client: 💔 Performance has regressed by +3.7647%. time: [210.60 ms 211.06 ms 211.64 ms]
thrpt: [472.50 MiB/s 473.79 MiB/s 474.84 MiB/s]
change:
time: [+3.4549% +3.7647% +4.1235] (p = 0.00 < 0.05)
thrpt: [-3.9602% -3.6281% -3.3396]
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high severetransfer/pacing-false/same-seed/wallclock-time/run: 💔 Performance has regressed by +3.3614%. time: [24.001 ms 24.025 ms 24.052 ms]
change: [+3.2390% +3.3614% +3.4794] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high severeAll results1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client: 💔 Performance has regressed by +2.5159%. time: [204.41 ms 204.75 ms 205.16 ms]
thrpt: [487.43 MiB/s 488.40 MiB/s 489.21 MiB/s]
change:
time: [+2.2390% +2.5159% +2.8188] (p = 0.00 < 0.05)
thrpt: [-2.7415% -2.4541% -2.1900]
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client: Change within noise threshold. time: [273.93 ms 275.79 ms 277.69 ms]
thrpt: [36.011 Kelem/s 36.260 Kelem/s 36.505 Kelem/s]
change:
time: [-2.9448% -1.9227% -0.9371] (p = 0.00 < 0.05)
thrpt: [+0.9459% +1.9604% +3.0342]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client: No change in performance detected. time: [38.430 ms 38.602 ms 38.798 ms]
thrpt: [25.774 B/s 25.905 B/s 26.021 B/s]
change:
time: [-0.5839% +0.0881% +0.6703] (p = 0.79 > 0.05)
thrpt: [-0.6659% -0.0880% +0.5874]
No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client: 💔 Performance has regressed by +3.7647%. time: [210.60 ms 211.06 ms 211.64 ms]
thrpt: [472.50 MiB/s 473.79 MiB/s 474.84 MiB/s]
change:
time: [+3.4549% +3.7647% +4.1235] (p = 0.00 < 0.05)
thrpt: [-3.9602% -3.6281% -3.3396]
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high severedecode 4096 bytes, mask ff: No change in performance detected. time: [6.8979 µs 6.9087 µs 6.9199 µs]
change: [-0.2537% +0.1030% +0.4814] (p = 0.62 > 0.05)
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severedecode 1048576 bytes, mask ff: Change within noise threshold. time: [1.7564 ms 1.7591 ms 1.7619 ms]
change: [-0.7694% -0.5487% -0.3262] (p = 0.00 < 0.05)
Change within noise threshold.decode 4096 bytes, mask 7f: No change in performance detected. time: [9.5001 µs 9.5273 µs 9.5648 µs]
change: [-0.4458% -0.0445% +0.3332] (p = 0.83 > 0.05)
No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
1 (1.00%) high mild
4 (4.00%) high severedecode 1048576 bytes, mask 7f: No change in performance detected. time: [2.4270 ms 2.4309 ms 2.4348 ms]
change: [-0.7128% -0.2208% +0.1834] (p = 0.37 > 0.05)
No change in performance detected.decode 4096 bytes, mask 3f: No change in performance detected. time: [11.169 µs 11.195 µs 11.220 µs]
change: [-2.6696% -0.9647% +0.0743] (p = 0.22 > 0.05)
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high milddecode 1048576 bytes, mask 3f: No change in performance detected. time: [2.8642 ms 2.8703 ms 2.8767 ms]
change: [-0.0536% +0.2080% +0.5006] (p = 0.14 > 0.05)
No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild1-streams/each-1000-bytes/wallclock-time: Change within noise threshold. time: [580.24 µs 582.24 µs 584.56 µs]
change: [-2.2539% -1.5993% -0.9317] (p = 0.00 < 0.05)
Change within noise threshold.
Found 7 outliers among 100 measurements (7.00%)
7 (7.00%) high severe1-streams/each-1000-bytes/simulated-time: No change in performance detected. time: [118.76 ms 118.98 ms 119.19 ms]
thrpt: [8.1931 KiB/s 8.2078 KiB/s 8.2227 KiB/s]
change:
time: [-0.4164% -0.1686% +0.0742] (p = 0.20 > 0.05)
thrpt: [-0.0741% +0.1689% +0.4181]
No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
1 (1.00%) low severe
6 (6.00%) low mild
2 (2.00%) high mild
1 (1.00%) high severe1000-streams/each-1-bytes/wallclock-time: No change in performance detected. time: [12.335 ms 12.371 ms 12.408 ms]
change: [-0.6668% +0.0021% +0.5832] (p = 1.00 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild1000-streams/each-1-bytes/simulated-time: No change in performance detected. time: [2.3283 s 2.3314 s 2.3346 s]
thrpt: [428.34 B/s 428.92 B/s 429.50 B/s]
change:
time: [-0.3602% -0.1417% +0.0767] (p = 0.21 > 0.05)
thrpt: [-0.0766% +0.1419% +0.3615]
No change in performance detected.1000-streams/each-1000-bytes/wallclock-time: No change in performance detected. time: [49.336 ms 49.436 ms 49.538 ms]
change: [-1.0273% -0.4733% -0.0261] (p = 0.05 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild1000-streams/each-1000-bytes/simulated-time: No change in performance detected. time: [16.184 s 16.463 s 16.739 s]
thrpt: [58.339 KiB/s 59.319 KiB/s 60.342 KiB/s]
change:
time: [-1.4641% +0.9659% +3.4745] (p = 0.45 > 0.05)
thrpt: [-3.3579% -0.9567% +1.4858]
No change in performance detected.coalesce_acked_from_zero 1+1 entries: No change in performance detected. time: [89.193 ns 89.557 ns 89.919 ns]
change: [-0.5570% -0.0354% +0.4406] (p = 0.89 > 0.05)
No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
9 (9.00%) high mild
3 (3.00%) high severecoalesce_acked_from_zero 3+1 entries: No change in performance detected. time: [106.13 ns 106.80 ns 107.77 ns]
change: [-0.4494% +0.0864% +0.7363] (p = 0.78 > 0.05)
No change in performance detected.
Found 13 outliers among 100 measurements (13.00%)
3 (3.00%) high mild
10 (10.00%) high severecoalesce_acked_from_zero 10+1 entries: No change in performance detected. time: [105.44 ns 105.89 ns 106.44 ns]
change: [-0.6622% -0.0020% +0.6072] (p = 1.00 > 0.05)
No change in performance detected.
Found 13 outliers among 100 measurements (13.00%)
5 (5.00%) low mild
1 (1.00%) high mild
7 (7.00%) high severecoalesce_acked_from_zero 1000+1 entries: No change in performance detected. time: [91.683 ns 94.334 ns 100.35 ns]
change: [-0.5171% +3.8641% +12.285] (p = 0.51 > 0.05)
No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severeRxStreamOrderer::inbound_frame(): Change within noise threshold. time: [109.82 ms 109.92 ms 110.04 ms]
change: [+0.1545% +0.2672% +0.3928] (p = 0.00 < 0.05)
Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
2 (2.00%) low mild
2 (2.00%) high mildsent::Packets::take_ranges: No change in performance detected. time: [4.4998 µs 4.7258 µs 5.0266 µs]
change: [-4.4114% -0.4961% +3.7743] (p = 0.82 > 0.05)
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severetransfer/pacing-false/varying-seeds/wallclock-time/run: Change within noise threshold. time: [23.686 ms 23.710 ms 23.736 ms]
change: [+0.3131% +0.5207% +0.6973] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severetransfer/pacing-false/varying-seeds/simulated-time/run: No change in performance detected. time: [23.941 s 23.941 s 23.941 s]
thrpt: [171.09 KiB/s 171.09 KiB/s 171.09 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/pacing-true/varying-seeds/wallclock-time/run: Change within noise threshold. time: [24.037 ms 24.071 ms 24.123 ms]
change: [+1.6454% +1.8826% +2.1454] (p = 0.00 < 0.05)
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) low mild
1 (1.00%) high mild
1 (1.00%) high severetransfer/pacing-true/varying-seeds/simulated-time/run: No change in performance detected. time: [23.676 s 23.676 s 23.676 s]
thrpt: [173.01 KiB/s 173.01 KiB/s 173.01 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/pacing-false/same-seed/wallclock-time/run: 💔 Performance has regressed by +3.3614%. time: [24.001 ms 24.025 ms 24.052 ms]
change: [+3.2390% +3.3614% +3.4794] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high severetransfer/pacing-false/same-seed/simulated-time/run: No change in performance detected. time: [23.941 s 23.941 s 23.941 s]
thrpt: [171.09 KiB/s 171.09 KiB/s 171.09 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.transfer/pacing-true/same-seed/wallclock-time/run: Change within noise threshold. time: [24.132 ms 24.158 ms 24.194 ms]
change: [+1.3886% +1.5240% +1.6956] (p = 0.00 < 0.05)
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) high mild
2 (2.00%) high severetransfer/pacing-true/same-seed/simulated-time/run: No change in performance detected. time: [23.676 s 23.676 s 23.676 s]
thrpt: [173.01 KiB/s 173.01 KiB/s 173.01 KiB/s]
change:
time: [+0.0000% +0.0000% +0.0000] (p = NaN > 0.05)
thrpt: [+0.0000% +0.0000% +0.0000]
No change in performance detected.Download data for |
Contributor
|
Bencher Report| Branch | feat-add_datagram-one-loop |
| Testbed | On-prem |
🚨 2 Alerts
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| decode 1048576 bytes, mask 3f | Latency milliseconds (ms) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 2.87 ms(+66.86%)Baseline: 1.72 ms | 2.27 ms (126.30%) |
| decode 4096 bytes, mask 3f | Latency microseconds (µs) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 11.20 µs(+56.58%)Baseline: 7.15 µs | 10.42 µs (107.46%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| 1-conn/1-100mb-req/mtu-1504 (aka. Upload)/client | 📈 view plot 🚷 view threshold | 211,060,000.00 ns(+1.57%)Baseline: 207,798,083.49 ns | 217,027,135.14 ns (97.25%) |
| 1-conn/1-100mb-resp/mtu-1504 (aka. Download)/client | 📈 view plot 🚷 view threshold | 204,750,000.00 ns(+1.35%)Baseline: 202,030,872.87 ns | 211,838,052.00 ns (96.65%) |
| 1-conn/1-1b-resp/mtu-1504 (aka. HPS)/client | 📈 view plot 🚷 view threshold | 38,602,000.00 ns(+13.27%)Baseline: 34,080,474.38 ns | 46,047,339.20 ns (83.83%) |
| 1-conn/10_000-parallel-1b-resp/mtu-1504 (aka. RPS)/client | 📈 view plot 🚷 view threshold | 275,790,000.00 ns(-4.58%)Baseline: 289,022,941.18 ns | 302,273,516.16 ns (91.24%) |
| 1-streams/each-1000-bytes/simulated-time | 📈 view plot 🚷 view threshold | 118,980,000.00 ns(+0.14%)Baseline: 118,811,537.00 ns | 120,522,203.25 ns (98.72%) |
| 1-streams/each-1000-bytes/wallclock-time | 📈 view plot 🚷 view threshold | 582,240.00 ns(-1.30%)Baseline: 589,911.82 ns | 610,765.47 ns (95.33%) |
| 1000-streams/each-1-bytes/simulated-time | 📈 view plot 🚷 view threshold | 2,331,400,000.00 ns(-75.81%)Baseline: 9,637,501,897.53 ns | 24,236,319,336.96 ns (9.62%) |
| 1000-streams/each-1-bytes/wallclock-time | 📈 view plot 🚷 view threshold | 12,371,000.00 ns(-7.51%)Baseline: 13,376,015.18 ns | 15,187,679.16 ns (81.45%) |
| 1000-streams/each-1000-bytes/simulated-time | 📈 view plot 🚷 view threshold | 16,463,000,000.00 ns(-7.84%)Baseline: 17,863,229,601.52 ns | 20,876,057,097.10 ns (78.86%) |
| 1000-streams/each-1000-bytes/wallclock-time | 📈 view plot 🚷 view threshold | 49,436,000.00 ns(-2.18%)Baseline: 50,536,466.79 ns | 55,784,046.58 ns (88.62%) |
| RxStreamOrderer::inbound_frame() | 📈 view plot 🚷 view threshold | 109,920,000.00 ns(+0.17%)Baseline: 109,728,387.10 ns | 111,386,019.03 ns (98.68%) |
| coalesce_acked_from_zero 1+1 entries | 📈 view plot 🚷 view threshold | 89.56 ns(+0.51%)Baseline: 89.10 ns | 90.40 ns (99.06%) |
| coalesce_acked_from_zero 10+1 entries | 📈 view plot 🚷 view threshold | 105.89 ns(-0.14%)Baseline: 106.04 ns | 107.12 ns (98.86%) |
| coalesce_acked_from_zero 1000+1 entries | 📈 view plot 🚷 view threshold | 94.33 ns(+3.79%)Baseline: 90.89 ns | 95.25 ns (99.03%) |
| coalesce_acked_from_zero 3+1 entries | 📈 view plot 🚷 view threshold | 106.80 ns(+0.24%)Baseline: 106.54 ns | 107.54 ns (99.31%) |
| decode 1048576 bytes, mask 3f | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 2,870,300.00 ns(+66.86%)Baseline: 1,720,207.78 ns | 2,272,645.56 ns (126.30%) |
| decode 1048576 bytes, mask 7f | 📈 view plot 🚷 view threshold | 2,430,900.00 ns(-51.04%)Baseline: 4,965,103.61 ns | 6,109,339.37 ns (39.79%) |
| decode 1048576 bytes, mask ff | 📈 view plot 🚷 view threshold | 1,759,100.00 ns(-40.89%)Baseline: 2,975,931.88 ns | 3,522,192.31 ns (49.94%) |
| decode 4096 bytes, mask 3f | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 11,195.00 ns(+56.58%)Baseline: 7,149.73 ns | 10,417.63 ns (107.46%) |
| decode 4096 bytes, mask 7f | 📈 view plot 🚷 view threshold | 9,527.30 ns(-50.84%)Baseline: 19,380.79 ns | 23,864.93 ns (39.92%) |
| decode 4096 bytes, mask ff | 📈 view plot 🚷 view threshold | 6,908.70 ns(-38.33%)Baseline: 11,202.98 ns | 13,355.34 ns (51.73%) |
| sent::Packets::take_ranges | 📈 view plot 🚷 view threshold | 4,725.80 ns(+0.87%)Baseline: 4,685.19 ns | 4,926.86 ns (95.92%) |
| transfer/pacing-false/same-seed/simulated-time/run | 📈 view plot 🚷 view threshold | 23,941,000,000.00 ns(-5.38%)Baseline: 25,302,323,809.52 ns | 26,117,166,039.80 ns (91.67%) |
| transfer/pacing-false/same-seed/wallclock-time/run | 📈 view plot 🚷 view threshold | 24,025,000.00 ns(-5.23%)Baseline: 25,349,838.10 ns | 27,046,230.04 ns (88.83%) |
| transfer/pacing-false/varying-seeds/simulated-time/run | 📈 view plot 🚷 view threshold | 23,941,000,000.00 ns(-4.75%)Baseline: 25,136,116,190.48 ns | 25,678,919,868.64 ns (93.23%) |
| transfer/pacing-false/varying-seeds/wallclock-time/run | 📈 view plot 🚷 view threshold | 23,710,000.00 ns(-7.02%)Baseline: 25,500,845.71 ns | 27,253,087.29 ns (87.00%) |
| transfer/pacing-true/same-seed/simulated-time/run | 📈 view plot 🚷 view threshold | 23,676,000,000.00 ns(-6.64%)Baseline: 25,361,188,571.43 ns | 26,314,846,327.31 ns (89.97%) |
| transfer/pacing-true/same-seed/wallclock-time/run | 📈 view plot 🚷 view threshold | 24,158,000.00 ns(-8.94%)Baseline: 26,528,899.05 ns | 28,761,848.43 ns (83.99%) |
| transfer/pacing-true/varying-seeds/simulated-time/run | 📈 view plot 🚷 view threshold | 23,676,000,000.00 ns(-5.09%)Baseline: 24,946,278,095.24 ns | 25,524,098,128.26 ns (92.76%) |
| transfer/pacing-true/varying-seeds/wallclock-time/run | 📈 view plot 🚷 view threshold | 24,071,000.00 ns(-7.43%)Baseline: 26,002,211.43 ns | 27,840,142.31 ns (86.46%) |
Collaborator
Author
|
Oh, nice |
Collaborator
Author
|
Closing in favor of #3171. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.