Contention-aware per-event send budget for QUICStream - #13554
Open
maskit wants to merge 7 commits into
Open
Conversation
A single stream on a QUIC connection could only send 16KB per write event, a fixed fairness quantum that protects sibling streams from being starved. That protection has a real cost when a connection has little or no contention: a single-stream, multi-megabyte response needs dozens of separate event-loop round trips to drain, each paying real per-call overhead, even though there's nobody else to be fair to. This scales the per-event budget by how many streams were writable in the previous event on the connection: down toward the original 16KB floor under real contention, up toward a new 256KB ceiling when a stream has the write path to itself. The count is remembered rather than recomputed each event, since quiche's writable-stream iterator is one-shot with no free count, and recomputing it would cost as much as the round trips being eliminated. A brief one-event lag when contention changes abruptly is bounded and self-correcting, never a starvation risk. Benchmarking a 1MB single-stream response over QUIC (h2load, -m 1) went from 812.7 MB/s to 1553.0 MB/s; a small (8KB) object, already under one turn's floor, was unaffected, confirming the fix targets exactly the large single-stream case without disturbing anything else.
Http3Transaction is constructed once per request and was allocating and freeing five separate objects on the heap for its own private, fixed-lifetime framers/handlers (header framer, data framer, protocol enforcer, header/data VIO adaptors) -- these become plain value members instead, owned directly by the transaction. Http3FrameHandler::interests() ran on every handler registration and constructed and returned a fresh std::vector by value each time, even though the interest set is fixed per handler type; it now returns a reference to a function-local static vector built once. Http3FrameDispatcher's per-type handler list moves from a std::vector (heap-allocated on first push) to a small fixed-size inline array sized for the real maximum handler count. Http3HeadersFrame gains a constructor that shares the caller's IOBufferReader via a clone instead of copying the header block into a freshly malloc'd buffer, used on the receive path where the block is already sitting in an IOBuffer. Unrelated one-line fix: QUICNetProcessor was checking dbg_ctl_vv_quiche with tag_on(), which only tests whether the debug tag pattern matches and ignores whether debug output is globally enabled -- switched to on() so a coincidental tag match doesn't install quiche's trace-level logger unconditionally.
QMuxConnection::_handle_write_streams() is the third of the three QUICStream::send_data() call sites, alongside QUICNetVConnection and OpenSSLQUICNetVConnection -- it was left on the fixed 16KB-per-event cap when the other two call sites moved to the contention-aware budget, since QUICStream::compute_fair_send_budget() didn't exist on the branch QMux's source lives on until this rebase brought both together. Same pattern as QUICNetVConnection::_handle_write_ready(): a _last_writable_stream_count member remembers the previous event's writable-stream count to size this event's budget, no extra allocation or extra quiche_conn_writable() call versus today's code.
Http3UnknownFrame was never instantiated anywhere -- no allocator for it exists (unlike every other frame type), and Http3FrameFactory::create()'s fallback for an unrecognized frame type constructs a plain Http3Frame directly instead. The whole subclass, both its constructors, and its to_io_buffer_block() override were orphaned code, predating this branch. Http3HeadersFrame's ats_unique_buf constructor and the create_headers_frame(const uint8_t*, size_t) factory overload that built it were also unreachable in production: Http3HeaderFramer.cc, their only real caller, has used the IOBufferReader*-based overload since this file's original introduction. The one place still exercising the owned-copy constructor was a unit test constructing it directly; removed along with it since the sibling "via factory" section already covers identical serialization output through the live construction path.
Http3DataFrame's send constructor takes IOBufferReader&, with its factory dereferencing a pointer to call it. Http3HeadersFrame's send constructor -- added when the reader-cloning path replaced the old copy-into-owned-buffer one -- took IOBufferReader* directly instead, with no actual need for pointer semantics: the constructor only calls clone() once, which works identically through a reference. Aligned it to the same &-parameter/factory-dereferences convention already used by every other IOBufferReader-taking constructor in this file (both frame types' receive-path constructors, and Data's send constructor).
_last_writable_stream_count was declared unconditionally but only read/written by the quiche write path, breaking the OpenSSL-native-QUIC build under -Werror=unused-private-field. Http3FrameDispatcher's new _handlers array lacked the same zero-initializer as its paired count array, a latent fragility if the count-gated read path is ever changed. Found in a deep review pass across both build backends.
maskit
commented
Aug 15, 2026
| const uint64_t total_len = this->_adapter->total_len(); | ||
|
|
||
| while (written_this_event < MAX_STREAM_SEND_BYTES_PER_EVENT) { | ||
| while (written_this_event < max_bytes_this_event) { |
Member
Author
There was a problem hiding this comment.
This is the biggest improvement. Fixed size cap -> Variable size cap based on the number of streams.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves QUIC/qmux write throughput for large single-stream responses by making QUICStream’s per-write-event send budget scale with observed connection contention (min 16KB under contention, up to 256KB when uncontended). It also reduces per-transaction overhead in the HTTP/3 frame path by removing dead code and eliminating several heap allocations.
Changes:
- Add contention-aware
QUICStream::compute_fair_send_budget()and plumb a per-event budget through QUICNetVConnection/qmux/OpenSSL QUIC write paths. - Reduce HTTP/3 per-transaction allocations by moving several
Http3Transactionhelpers to value members and switchingHttp3FrameDispatcherto fixed inline handler storage. - Remove dead HTTP/3 frame code and make HEADERS frame send-path avoid copying by cloning an
IOBufferReader.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/proxy/http3/test/test_Http3FrameDispatcher.cc | Update test handler interests() to return a stable const& list. |
| src/proxy/http3/test/test_Http3Frame.cc | Update HEADERS frame test to use reader-based factory path. |
| src/proxy/http3/test/Mock.h | Update mock handler interests() to return a stable const& list. |
| src/proxy/http3/Http3Transaction.cc | Convert several HTTP/3 helpers from heap pointers to value members; adjust call sites. |
| src/proxy/http3/Http3StreamDataVIOAdaptor.cc | Return stable interests() list; make has_data() const. |
| src/proxy/http3/Http3SettingsHandler.cc | Return stable interests() list by const&. |
| src/proxy/http3/Http3ProtocolEnforcer.cc | Return stable interests() list by const&. |
| src/proxy/http3/Http3HeaderVIOAdaptor.cc | Return stable interests() list by const&. |
| src/proxy/http3/Http3FrameDispatcher.cc | Replace per-type std::vector with inline handler storage; remove per-dispatch copies. |
| src/proxy/http3/Http3FrameCounter.cc | Return stable interests() list by const&. |
| src/proxy/http3/Http3Frame.cc | Remove unknown-frame implementation; make send-path HEADERS frame use cloned reader; remove dead factory overload. |
| src/iocore/net/unit_tests/test_QUICStream.cc | Add unit tests for compute_fair_send_budget() edge cases. |
| src/iocore/net/QUICNetVConnection.cc | Compute per-event stream send budget from previous writable-stream count; pass budget into send_data(). |
| src/iocore/net/QUICNetProcessor.cc | Use DbgCtl::on() instead of tag_on() when enabling quiche debug logging. |
| src/iocore/net/quic/QUICStream.cc | Implement compute_fair_send_budget() and budgeted send_data() loop. |
| src/iocore/net/qmux/QMuxConnection.cc | Compute per-event stream send budget from previous writable-stream count; pass budget into send_data(). |
| src/iocore/net/P_QUICNetVConnection.h | Track previous writable-stream count for quiche path budget computation. |
| src/iocore/net/OpenSSLQUICNetVConnection.cc | Update send_data() calls to new signature (using min budget for legacy behavior). |
| src/iocore/net/CMakeLists.txt | Add new QUICStream unit test to test_net when QUIC or qmux is enabled. |
| include/proxy/http3/Http3Transaction.h | Include concrete HTTP/3 helper types and store helpers as value members. |
| include/proxy/http3/Http3StreamDataVIOAdaptor.h | Update interests() return type; make has_data() const. |
| include/proxy/http3/Http3SettingsHandler.h | Update interests() return type. |
| include/proxy/http3/Http3ProtocolEnforcer.h | Update interests() return type. |
| include/proxy/http3/Http3HeaderVIOAdaptor.h | Update interests() return type. |
| include/proxy/http3/Http3FrameHandler.h | Change interests() interface to return std::vector<...> const &. |
| include/proxy/http3/Http3FrameDispatcher.h | Switch to inline handler storage and add max-handlers limit. |
| include/proxy/http3/Http3FrameCounter.h | Update interests() return type. |
| include/proxy/http3/Http3FrameCollector.h | Reserve fixed generator capacity to avoid vector growth reallocations. |
| include/proxy/http3/Http3Frame.h | Remove unknown frame type; add reader-backed HEADERS send-path constructor; remove dead factory overload. |
| include/iocore/net/quic/QUICStream.h | Add min/max per-event budgets; expose compute_fair_send_budget() and new send_data() signature. |
| include/iocore/net/qmux/QMuxConnection.h | Track previous writable-stream count for qmux budget computation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The test freed the source MIOBuffer while the frame it produced was still alive, so the frame's destructor deallocated a reader into already-freed memory when it went out of scope. Reset the frame before freeing the buffer, per the constructor's own documented lifetime requirement.
bneradt
reviewed
Aug 17, 2026
| const uint64_t total_len = this->_adapter->total_len(); | ||
|
|
||
| while (written_this_event < MAX_STREAM_SEND_BYTES_PER_EVENT) { | ||
| while (written_this_event < max_bytes_this_event) { |
Contributor
There was a problem hiding this comment.
[P2] Cap a carried pending block to this event’s remaining budget.
_pending_send_block can have been created under a previous 256 KB budget and survive a partial write. If contention then reduces max_bytes_this_event to 16 KB, the later write_stream() call still passes the entire remaining block, allowing the stream to exceed the computed budget. Please cap the submitted length to max_bytes_this_event - written_this_event (and only set fin when the whole pending block is included). A unit test with a partial first write followed by a smaller budget would cover this transition.
| QUICNetVConnection::_handle_write_ready() | ||
| { | ||
| if (quiche_conn_is_established(this->_quiche_con)) { | ||
| const size_t budget = QUICStream::compute_fair_send_budget(this->_last_writable_stream_count); |
Contributor
There was a problem hiding this comment.
[P2] Avoid using the maximum budget for every stream on the first event.
_last_writable_stream_count starts at 1, so an event whose current iterator contains many streams gives all of them the 256 KB maximum. With the configured 100-stream limit, that can queue or process roughly 25.6 MB in one event instead of the previous 1.6 MB, delaying unrelated connections on the same event thread. The QMux path has the same pattern. Please initialize conservatively, enforce a connection-wide per-event cap, or otherwise account for current contention.
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.
Summary
Benchmarking qmux (HTTP/3-over-TCP) against H2 showed a throughput deficit
specific to large, single-stream responses, absent for small objects and
mostly recovered once multiple concurrent streams share a connection.
QUICStream::send_data()capped how much of one stream's data it sendsper write event to a fixed 16KB, regardless of contention. A large object
on a solo stream needs many separate event-loop round trips to drain,
each paying real per-event overhead (quiche iterator alloc/free, an FFI
crossing, a flush). This makes that per-event budget contention-aware:
it scales down toward the existing 16KB floor under real contention
(preserving today's fairness guarantee exactly), and grows toward a new
256KB ceiling when a stream has the connection's write path to itself.
The budget is sized from the writable-stream count observed during the
previous write event on that connection -- an intentional one-event-lag
tradeoff that avoids draining
quiche_conn_writable()twice ormaterializing a container just to count entries.
Also, while touching the HTTP/3 frame-handling path, removed some dead
code and per-request allocations found along the way:
Http3FrameDispatcher's per-type handler list moved from aheap-allocated
std::vectorto fixed inline storage, since at most ahandful of handlers ever register per frame type and this dispatcher
is a per-transaction object.
Http3Transaction's five per-transaction helper objects (framegenerators/handlers) converted from heap-allocated pointers to value
members, removing five allocations/frees per HTTP/3 transaction.
Http3UnknownFrameandHttp3HeadersFrame'sats_unique_bufconstructor/factory overload, both fully dead code.
Http3HeadersFrame's send-path constructor to takeIOBufferReader &likeHttp3DataFrame's equivalent, instead of*.Test plan
test_QUICStream.cc) coverscompute_fair_send_budget()'s no-contention, heavy-contention,mid-range, and floor-boundary cases.
test_netandtest_http3pass against BoringSSL + quiche + qmux.test_netandtest_http3pass against OpenSSL's native QUICbackend (a separate, non-quiche
NetVConnectionimplementationthat also compiles this code).