Scale receive-window ACK replenishment with the window size - #1123
Conversation
Optimizing flow control trades one scenario against another, so a single bulk-throughput number is not enough to justify a change. This adds four benchmarks that cover the ways those trades show up: - BulkTransferBenchmark: one channel, large one-way payload. The scenario from #505, and the one dominated by flow control. Parameterized by window size, because a small window stresses the acknowledgement path far harder than the default and is the case most likely to regress when the acknowledgement policy changes. - DuplexMessagingBenchmark: small-message round trips, as StreamJsonRpc generates. Messages never fill a window, so this isolates the latency of a single frame's trip through the send path, which throughput numbers hide. - ContendedChannelsBenchmark: several channels transmitting at once. All frames on a connection share one send path, so this is where contention there becomes visible. - ChannelLifetimeBenchmark: opening and closing many channels that carry little data, which is the shape of a large IDE session. Paired with the memory diagnoser, it guards against buying throughput by making every channel more expensive to own. All four take ProtocolMajorVersion as a parameter so that v1, which has no backpressure, serves as the ceiling that the cost of flow control in v2/v3 can be measured against. Two supporting changes: The benchmarks run in-process. The default toolchain generates and builds a separate project per benchmark, which fails here when the installed SDK is newer than the target framework. Process isolation buys little for an operation that runs for tens of milliseconds and is dominated by socket I/O. SequenceBenchmark is unsealed. BenchmarkDotNet cannot run benchmarks in a sealed class, so it was silently being skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The receiving side previously sent a ContentProcessed (ACK) frame after every 20KB (one max frame payload) of examined content, regardless of how large the receiving window was. With the 1MB default window this meant ~50 ACK frames per window, each one taking the send lock, serializing a frame, and flushing the transport. Scale the ACK threshold with the window instead: ACK once 1/8 of the window has been examined, with a floor of one max frame payload so small windows behave exactly as before. One eighth still replenishes credit long before the sender can exhaust the window, so the sender never stalls. The exact `bytesProcessed == localWindowSize` safety net is preserved. It is what guarantees progress when the window is smaller than the frame-payload floor, and dropping it deadlocks every backpressure test. Measured on loopback TCP, 32MB transfers, 15 iterations: | Benchmark | before | after | |----------------------------|---------|---------| | BulkTransfer v2, 1MB window | 117.8 ms | 64.5 ms | | BulkTransfer v3, 1MB window | 102.4 ms | 59.4 ms | | Contended v2, 8 channels | 96.9 ms | 60.5 ms | | Contended v3, 8 channels | 114.3 ms | 49.8 ms | Allocations drop with it (v3 bulk 896KB -> 715KB, v3 contended 1764KB -> 1418KB), which is the expected signature of sending 8x fewer ACK frames. The 100KB-window cases are statistically unchanged (100KB/8 clamps up to the 20KB floor, so the behavior is identical), and DuplexMessaging and ChannelLifetime show no regression. This is a receiver-side policy change only. No wire format change, so it is fully compatible with existing v2 and v3 peers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Added WhyEvery existing benchmark here 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: 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.
The injected latency reproduces theory closely, which is the main evidence the harness measures what it claims: with a 1MB window and a 16ms RTT, window-limited throughput predicts Results (4MB transfer, 15 iterations)
At a 16ms round trip the 1MB default window costs v2/v3 a 2.9x penalty against v1, and a 4MB window erases it entirely. v1 is unaffected throughout since it has no backpressure. Context: what this settlesI also explored pipelined sends (deferring the transport flush so a sender need not wait a round trip per frame). Instrumenting the actual stalls showed why. Sender stalls waiting for credit were only ~6.6% of runtime, while throughput tracked
That works out to roughly 12 thread context switches per ACK frame — the real cost, and the reason Loopback therefore says "return credit far less often." The 8ms rows above say why that must not be done: at a realistic RTT the window is already the binding constraint before credit granularity matters, so coarser credit would make those rows worse. Hence this benchmark, before any divisor change. Nothing in the library changed in this commit — it is benchmark infrastructure only. 889 tests pass. |
Follow-up to #1119 (which addressed #505). This adds a benchmark matrix to measure
MultiplexingStreamunder several workload shapes, then uses it to land one flow-control change and to reject another.e9bd2ab— benchmark matrixFour benchmarks, each parameterized over protocol v1/v2/v3 so regressions and wins are attributable to a specific protocol version:
They share a loopback-TCP harness so the transport is realistic rather than in-memory.
This also removes
sealedfromSequenceBenchmark. BenchmarkDotNet cannot run benchmarks in a sealed class, so that benchmark had been silently skipped — a pre-existing bug.4d24152— replenish the receive window at 1/8 of its sizeThe receiver previously sent a
ContentProcessed(ACK) frame after every 20KB (one max frame payload) of examined content, regardless of window size. With the 1MB default window that is ~50 ACK frames per window, each one taking the send lock, serializing a frame, and flushing the transport.The ACK threshold now scales with the window — ACK once 1/8 of the window has been examined — with a floor of one max frame payload so small windows behave exactly as before. One eighth still replenishes credit long before the sender can exhaust the window, so the sender never stalls.
The exact
bytesProcessed == localWindowSizesafety net is preserved. It is what guarantees progress when the window is smaller than the frame-payload floor; dropping it deadlocks every backpressure test.Measured on loopback TCP, 32MB transfers, 15 iterations:
These are 5–10σ effects, and the baseline was re-measured afterward to confirm it reproduced. Allocations fell with them (v3 bulk 896KB → 715KB, v3 contended 1764KB → 1418KB), which is the expected signature of sending 8x fewer ACK frames. The 100KB-window cases are statistically unchanged, as predicted — 100KB/8 clamps up to the 20KB floor, so behavior there is identical. DuplexMessaging and ChannelLifetime show no regression.
This is a receiver-side policy change only. No wire format change, so it is fully compatible with existing v2 and v3 peers, including the JS implementation.
Tests
Two tests added (in
MultiplexingStreamV2Tests, so they run for both v2 and v3):Backpressure_CreditIsReturnedForAnyWindowSizeacross windows of 1KB, 20KB, 100KB, and 1MB.Backpressure_SmallReadsStillMakeProgress— a receiver examining in 4KB increments against a 1MB window, the shape most likely to stall if the receiver waits for too much data before acknowledging.Both were mutation-tested: with a deliberately broken threshold, all 10 cases deadlock.
Rejected: replacing the send semaphore with an MPSC queue
I also implemented the "single-writer send loop" idea —
System.Threading.ChannelsMPSC queue draining into one writer that flushes once per batch instead of once per frame — to amortize flushes. It worked (all tests green, including new ones for cross-channel ordering, disposal with a queued backlog, and fault propagation to senders), but it measured as a regression, so it is not included here:AllowSynchronousContinuations = truedid not rescue it.The reason batching cannot pay off in the current design:
ProcessOutboundTransmissionsAsyncawaits each frame before it may release the payload buffer back to the pipe, so the queue never holds more than one content frame — the batch size is always 1. All the change adds is a queue hop and a completion source per frame, replacing an uncontended semaphore that completed synchronously.Two findings from that exercise are worth recording even though the code is reverted:
openChannels/channelsPendingTerminationchecks must happen at enqueue time, in the caller's causal order. Deferring them into the send loop lets unrelated senders reorder relative to a channel termination, which broke the protocol in two existing tests.Offerframes whose token was canceled after the send began, leaving the peer with content frames for a channel it never saw offered.Flush batching would only pay off alongside pipelined sends (several outstanding frames per channel), which requires decoupling payload lifetime from
AdvanceTo. That is a larger change and is not attempted here.Protocol v4: auto-tuning receiving windows
v2/v3 fix a channel's receiving window 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 idle channel expensive.
v4 lets a window grow after the fact, but only for channels that demonstrably need it. Growth needs evidence from both ends:
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 v4 exists to fix.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. Consecutive refusals back the sender off exponentially, so a consumer-limited channel stops spending frames on requests that won't be granted.
Growth is ×4, bounded per channel by
Options.MaxChannelReceivingWindowSizeand per connection byOptions.MaxTotalChannelReceivingWindowSize; budget is returned when a channel closes. Adaptive sizing is purely receiver-side policy — the wire contract is just "you may send N more bytes" — so an implementation that never grows a window is still a conforming v4 peer (relevant to #1116).Measurements (ms, lower is better; 4 MB transfer unless noted)
Roughly 2x over v3 wherever the window was the constraint, allocations flat or lower, no regression elsewhere. The residual gap to v1 under latency is about one round trip: the first stall is how a channel learns it needs a bigger window, so some ramp is unavoidable.
Known follow-ups