Conversation
jesup
requested review from
KershawChang,
larseggert,
martinthomson and
mxinden
as code owners
September 14, 2026 02:23
This was referenced Sep 14, 2026
Member
Author
|
This PR is part of a stack of 10 bookmarks:
Created with jj-stack |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The required public trait method is source-breaking, and the new test documentation misidentifies the queue layer.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Exposes per-session WebTransport outgoing-datagram queue capacity for content-process credit management.
Changes:
- Adds a client-facing capacity query.
- Makes internal capacity lookup available in production.
- Tests queue accounting beyond the legacy FIFO size.
File summaries
| File | Description |
|---|---|
neqo-http3/src/webtransport.rs |
Adds the client capacity API and delegation. |
neqo-http3/src/connection.rs |
Adds validated WebTransport capacity lookup. |
neqo-http3/src/features/extended_connect/session.rs |
Enables production access to session capacity. |
neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs |
Tests per-session queue capacity accounting. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## users/jesup/drop_datagrams_on_teardown #3993 +/- ##
=======================================================================
Coverage 96.93% 96.93%
=======================================================================
Files 119 119
Lines 41579 41584 +5
Branches 41579 41584 +5
=======================================================================
+ Hits 40303 40309 +6
+ Misses 1252 1251 -1
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | walltime/1-streams/each-4194304-bytes |
10.6 ms | 11 ms | -2.98% |
| ⚡ | Simulation | coalesce_acked_from_zero 1 ranges |
2.8 µs | 2.7 µs | +3.96% |
| ⚡ | Simulation | coalesce_acked_from_zero 3 ranges |
3.7 µs | 3.6 µs | +2.98% |
| ⚡ | WallTime | neqo-neqo-cubic |
20.9 ms | 20.3 ms | +2.68% |
| ⚡ | WallTime | 1-conn/1-1b-resp (aka. HPS) |
3 ms | 2.9 ms | +2.07% |
| ⚡ | WallTime | 1-conn/1-100mb-resp (aka. Download) |
48.2 ms | 47.3 ms | +2.05% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing users/jesup/expose_queue_capacity (f333aef) with users/jesup/drop_datagrams_on_teardown (dc976e1)
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 14, 2026 05:31
0436765 to
6d838db
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 14, 2026 05:31
e4297ad to
b3cb781
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 14, 2026 17:56
6d838db to
e919805
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 14, 2026 17:56
b3cb781 to
f686bf9
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 16, 2026 03:45
e919805 to
6c8995d
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 16, 2026 03:45
f686bf9 to
816d884
Compare
mxinden-bot
pushed a commit
to mxinden-bot/neqo
that referenced
this pull request
Sep 16, 2026
Written against the tip of the per-session datagram queue stack (mozilla#3993). All four fail; the other 943 neqo-transport lib tests pass. DatagramQueue::enqueue never expires, so a datagram already past its max age still counts against the byte budget and the high water mark. It is reported Dropped rather than Expired, mis-attributing the stat and giving the application backpressure whose real cause was age, not depth. The previous stack fixed this in Session::send_datagram; the rewrite lost it. Fixing it in place needs default_max_age stored on the queue rather than threaded in as a parameter. QuicDatagrams::set_datagram_high_water_mark does not call resume_if_unblocked, unlike set_datagram_max_age right above it, so an application that raises outgoingMaxBufferedDatagrams while blocked waits until a send or an expiry happens to revisit the queue. A high water mark of zero wedges the sender permanently: below_watermark is total_count < mark, false at every count, so resume_if_unblocked can never fire. Arguably faithful to a degenerate corner of the spec, so this one is a question rather than a defect.
mxinden
reviewed
Sep 16, 2026
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 20, 2026 19:34
aebb346 to
1e99e70
Compare
This was referenced Sep 20, 2026
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 23, 2026 06:27
c0eec0c to
609dbad
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 23, 2026 06:27
1e99e70 to
1cb9ec5
Compare
Contributor
Performance profiles for profiler.firefox.comBenchmarks (14)
|
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 23, 2026 23:55
609dbad to
f3f6e7c
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 23, 2026 23:55
1cb9ec5 to
b172928
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 25, 2026 10:28
f3f6e7c to
b3f7e86
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 25, 2026 10:28
b172928 to
c0707d8
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 25, 2026 12:35
b3f7e86 to
fad8dba
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 25, 2026 12:35
c0707d8 to
c245d30
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
September 25, 2026 15:05
fad8dba to
37fdffd
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
2 times, most recently
from
September 30, 2026 03:30
419a6ae to
5a2f7b1
Compare
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
2 times, most recently
from
September 30, 2026 15:32
5f27707 to
0415ed4
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
September 30, 2026 15:32
5a2f7b1 to
41cc958
Compare
larseggert
approved these changes
Sep 30, 2026
Add a production-facing Http3Client::webtransport_datagram_queue_capacity, so a caller (e.g. a content-process credit grant) can read a session's outgoing-datagram queue state without going through the test-only path. The underlying transport-level DatagramQueueCapacity snapshot and per-session accessor already existed but were only reachable from #[cfg(test)] code; drop that gate now that there's a real caller.
jesup
force-pushed
the
users/jesup/drop_datagrams_on_teardown
branch
from
October 2, 2026 21:48
0415ed4 to
dc976e1
Compare
jesup
force-pushed
the
users/jesup/expose_queue_capacity
branch
from
October 2, 2026 21:48
41cc958 to
f333aef
Compare
Contributor
Client/server transfer resultsPerformance differences relative to dc976e1. 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
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Contributor
Benchmark resultsNo significant performance differences relative to dc976e1. All resultstransfer/1-conn/1-100mb-req (aka. Upload)/mtu-1500 time: [50.275 ms 50.311 ms 50.349 ms]
thrpt: [1.9396 GiB/s 1.9411 GiB/s 1.9425 GiB/s]
Found 8 outliers among 100 measurements (8.00%)
3 (3.00%) low mild
4 (4.00%) high mild
1 (1.00%) high severetransfer/1-conn/1-100mb-resp (aka. Download)/mtu-1500 time: [49.505 ms 49.562 ms 49.621 ms]
thrpt: [1.9681 GiB/s 1.9704 GiB/s 1.9727 GiB/s]
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/1-conn/1-1b-resp (aka. HPS)/mtu-1500 time: [3.0143 ms 3.0151 ms 3.0159 ms]
thrpt: [331.58 B/s 331.66 B/s 331.75 B/s]
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mildtransfer/1-conn/10_000-parallel-1b-resp (aka. RPS)/mtu-1500 time: [68.304 ms 68.560 ms 68.817 ms]
thrpt: [145.31 Kelem/s 145.86 Kelem/s 146.40 Kelem/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams-flow-controlled/walltime/1-streams/each-4194304-bytes time: [11.038 ms 11.040 ms 11.043 ms]
thrpt: [362.22 MiB/s 362.31 MiB/s 362.40 MiB/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams-flow-controlled/walltime/10-streams/each-1048576-bytes time: [28.101 ms 28.110 ms 28.119 ms]
thrpt: [355.63 MiB/s 355.75 MiB/s 355.86 MiB/s]
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildstreams/walltime/1-streams/each-1000-bytes time: [131.63 µs 132.17 µs 132.87 µs]
thrpt: [7.1777 MiB/s 7.2153 MiB/s 7.2449 MiB/s]
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildstreams/walltime/1000-streams/each-1-bytes time: [3.8203 ms 3.8323 ms 3.8439 ms]
thrpt: [254.06 KiB/s 254.82 KiB/s 255.63 KiB/s]
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) low mildstreams/walltime/1000-streams/each-1000-bytes time: [11.223 ms 11.227 ms 11.231 ms]
thrpt: [84.917 MiB/s 84.945 MiB/s 84.974 MiB/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-false/same-seed time: [2.0144 ms 2.0158 ms 2.0174 ms]
thrpt: [1.9362 GiB/s 1.9378 GiB/s 1.9392 GiB/s]transfer/walltime/pacing-false/varying-seeds time: [2.0127 ms 2.0137 ms 2.0151 ms]
thrpt: [1.9385 GiB/s 1.9398 GiB/s 1.9408 GiB/s]transfer/walltime/pacing-true/same-seed time: [2.1079 ms 2.1100 ms 2.1125 ms]
thrpt: [1.8491 GiB/s 1.8513 GiB/s 1.8531 GiB/s]transfer/walltime/pacing-true/varying-seeds time: [2.0628 ms 2.0636 ms 2.0646 ms]
thrpt: [1.8920 GiB/s 1.8929 GiB/s 1.8936 GiB/s]
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildInstructions per cycleCriterion reported no significant timing changes. All benchmarks
Profiles for profiler.firefox.com (62)
Download data for |
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.
Add
ClientSession::webtransport_datagram_queue_capacity, so a caller (Firefox's content-process credit grant) can read a WebTransport session's outgoing-datagram queue state. The transport'sDatagramQueueCapacitysnapshot and the per-session accessor already existed behind#[cfg(test)]; this drops that and validates the session as WebTransport, so a connect-udp session id is rejected.