Grow channel receiving windows on demand - #1124
Merged
Merged
Conversation
Every existing MultiplexingStream benchmark runs over loopback, where a round trip is essentially free. That makes them blind to half of the central trade-off in the receive window's flow control: returning credit less often costs fewer frames, but makes the sender wait a full round trip when it does run out. Loopback only shows the first half, so tuning against it alone will happily pick a setting that behaves badly on a real network. `LatencyStream` wraps a transport and withholds arriving data for a fixed one-way delay. Wrapping both ends gives a round trip time of twice that delay. Delayed data is held in a queue and released by a background pump so transfers stay pipelined; simply awaiting before returning from each read would serialize the connection and cap throughput at one chunk per delay, measuring the harness instead of the library. `MultiplexingStreamBenchmarkBase` gains a `WrapTransport` hook so this and any future transport decoration can be applied without duplicating the setup. The injected latency reproduces theory closely, which is the main evidence that the harness measures what it claims. With a 1MB window and a 16ms round trip, window-limited throughput predicts 4MB / (1MB / 16ms) = 64ms of stall; the measured penalty over the v1 floor is 63ms. Results (4MB transfer, 15 iterations): | one-way | window | v1 | v2 | v3 | |---------|---------|-------|-------|-------| | 0 ms | default | 6.8 | 12.6 | 13.5 | | 1 ms | default | 12.3 | 17.3 | 17.4 | | 8 ms | default | 32.5 | 95.1 | 94.8 | | 8 ms | 4 MB | 32.1 | 31.7 | 31.6 | The headline is the last two rows. At a 16ms round trip the 1MB default window costs v2/v3 a 2.9x penalty against v1, and raising the window to 4MB erases it entirely, reaching parity. v1 is unaffected throughout because it has no backpressure at all. This also settles a question the loopback benchmarks could not. Loopback measurements suggest returning credit far less often is a large win, because there each ContentProcessed frame costs roughly a dozen thread context switches and stalls are nearly free. These results show why that must not be tuned on loopback: at a realistic round trip the window is already the binding constraint before credit granularity even matters, so coarser credit would make the 8ms rows worse, not better. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A channel's receiving window is fixed when the channel is accepted, and that one number has to serve two irreconcilable purposes: bounding how much unread data a receiver may buffer, and bounding how much data a sender may keep in flight. On a link with real latency the second dominates -- 1 MB over a 16 ms round trip caps a channel at ~62 MB/s no matter how fast either end is -- but raising the default to suit it would make every channel's worst case more expensive. A window may now grow after the fact, but only for the channels that demonstrably need it. Growth requires evidence from both ends: - The sender reports that it ran out of credit with more to send (ChannelWindowGrowthRequest). Only the sender can observe this; a receiver that drains promptly never sees its own buffer fill even while the sender sits blocked across the round trip. - The receiver grants only while its own reader is starved inside ReadAsync (ChannelWindowAdjust). If the reader is merely behind, a larger window would buffer more unread data without moving any more of it. A request that arrives while the receiver is busy is deferred and answered the next time its reader starves. Every request is answered, including a refusal that repeats the current size, so the sender is never left waiting on a reply that will not come. Consecutive refusals back the sender off exponentially, so a channel already at its cap does not spend a frame pair on every subsequent stall. This needs no new protocol version. Both control codes are additive: the .NET and JS frame dispatchers both end in a `default:` case that ignores unrecognized codes, and frames are self-delimiting, so an older peer skips them cleanly. The exchange is also self-negotiating, because a receiver only ever grants in reply to a request, and only a peer that implements this sends one. Against an older peer a channel spends one small frame that is ignored, then behaves exactly as a fixed window would. MultiplexingStreamWindowFrameCompatTests proves this by relaying a real connection through a filter that drops both frame types, and asserts that frames really were dropped so the test cannot pass vacuously. Growth is by a factor of 4, bounded per channel by Options.MaxChannelReceivingWindowSize and across the connection by Options.MaxTotalChannelReceivingWindowSize; budget is returned when a channel closes. Note that a window is a limit on buffering rather than an allocation -- it is used only as a Pipe pauseWriterThreshold and to size ACKs -- so a channel that never fills one costs nothing. Measured with LatencyBulkTransferBenchmark (4 MB transfer): | one-way latency | window | v1 | before | after | |-----------------|---------|------:|-------:|------:| | 0 ms | default | 6.7 | 13.3 | 13.5 | | 1 ms | default | 11.4 | 17.5 | 13.5 | | 8 ms | default | 33.2 | 94.6 | 47.7 | | 8 ms | 4 MB | 32.1 | 31.7 | 31.6 | Reproduced across separate runs with errors near +/-1 ms. The loopback benchmarks are inconclusive here and are deliberately not claimed as evidence: the same binary run twice gave 54.95 ms and 34.58 ms for the same case, so they cannot resolve an effect of this size on this machine. Instrumenting a 32 MB loopback transfer showed zero growth requests, because a prompt reader at zero latency never lets the sender stall -- so the feature is simply inert there, neither helping nor hurting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Follow-up to #1123, continuing the work on #505.
A channel's receiving window is fixed when the channel is accepted, and that one number has to serve two irreconcilable purposes: bounding how much unread data a receiver may buffer, and bounding how much data a sender may keep in flight. On a link with real latency the second dominates — 1 MB over a 16 ms round trip caps a channel at ~62 MB/s regardless of how fast either end is — but raising the default to suit it makes every channel's worst case more expensive.
This PR lets a window grow after the fact, but only for the channels that demonstrably need it.
How growth is decided
Growth requires evidence from both ends, which is what keeps it from degenerating into "buffer more, always":
ChannelWindowGrowthRequest). Only the sender can observe this. A receiver that drains promptly never sees its own buffer fill, even while the sender sits blocked across the round trip — which is precisely the case this fixes. (An earlier attempt at receiver-side saturation detection never fired, for exactly this reason.)ReadAsync(ChannelWindowAdjust). If the reader is merely behind, a bigger window would buffer more unread data without moving any more of it.A request arriving while the receiver is busy is deferred and answered when its reader next starves. Every request is answered, including a refusal that repeats the current size, so the sender is never left waiting on a reply that won't come. Consecutive refusals back the sender off exponentially, so a channel already at its cap doesn't spend a frame pair on every subsequent stall.
Growth is ×4, bounded per channel by
Options.MaxChannelReceivingWindowSizeand per connection byOptions.MaxTotalChannelReceivingWindowSize; budget is returned when a channel closes.Worth noting explicitly: a window is a limit on buffering, not an allocation.
localWindowSizeis used only as aPipepauseWriterThresholdand to size ACKs, so a channel that never fills one costs nothing.No new protocol version
The two control codes are purely additive:
default:case that ignores unrecognized codes (MultiplexingStream.cs,MultiplexingStream.ts), and(ControlCode)reader.ReadInt32()is a plain cast that never throws.Rather than assert this,
MultiplexingStreamWindowFrameCompatTestsrelays a real connection through a filter that drops both frame types, and asserts that frames really were dropped so the test can't pass vacuously.Measurements
LatencyBulkTransferBenchmark, 4 MB transfer, ms (lower is better):Reproduced across separate runs with errors near ±1 ms. The residual gap to v1 at 8 ms is about one round trip: the first stall is how a channel learns it needs a bigger window, so some ramp is unavoidable.
The loopback benchmarks are inconclusive on my machine and I'm deliberately not claiming them as evidence. The same binary run twice gave 54.95 ms and 34.58 ms for the same case. Instrumenting a 32 MB loopback transfer showed zero growth requests — a prompt reader at zero latency never lets the sender stall — so the feature is simply inert there, neither helping nor hurting. Worth re-running the matrix on quieter hardware.
Known follow-ups