Skip to content

fix: preserve connection task coalescing#920

Closed
brentechols wants to merge 3 commits into
hyperium:sean/bye-bye-big-mutexfrom
brentechols:fix-pr917-window-update-coalescing
Closed

fix: preserve connection task coalescing#920
brentechols wants to merge 3 commits into
hyperium:sean/bye-bye-big-mutexfrom
brentechols:fix-pr917-window-update-coalescing

Conversation

@brentechols

Copy link
Copy Markdown

This is stacked on top of #917.

Problem

In #917, Inner::poll_complete registers the connection task waker before draining pending stream operations. That lets stream handles wake the connection task while it is still actively polling the current batch of work. For receive-side capacity releases, this changes the old batching boundary from main: instead of accumulating releases until the current poll has drained, the connection can be woken repeatedly and emit smaller, more frequent WINDOW_UPDATE frames.

In the reproducer this showed up most clearly with one TCP connection, default flow-control windows, and multiple concurrent streams. Base #917 sent roughly twice as many connection-level WINDOW_UPDATE frames as main/fixed, with about half the average update size.

Solution

Delay shared.conn_task.register(cx.waker()) until after recv/send pending work has been drained, matching the old single-waker coalescing boundary. The pending paths still register the waker before returning Poll::Pending, so transport wakeups are not missed.

Validation

Focused test suite:

cargo test --offline -p h2-tests --test flow_control
47 passed; 4 ignored

Benchmark configuration:

requests=300
response_size=1048576
stream_window_size=65535
connection_window_size=65535
worker_threads=8
TCP_NODELAY=false
connections=1,2,4,8
streams_per_connection=1,2,4,8

Single-run Windows loopback results:

Conns Streams Total conc. Main req/s Base #917 req/s Fixed req/s Fixed vs base Fixed vs main
1 1 1 1,090.3 48.2 1,030.4 21.37x 95%
1 2 2 1,010.0 69.3 1,100.3 15.89x 109%
1 4 4 1,041.1 71.7 1,050.7 14.65x 101%
1 8 8 736.2 81.5 753.3 9.25x 102%
2 1 2 682.2 145.9 742.3 5.09x 109%
2 2 4 593.9 282.5 566.4 2.01x 95%
2 4 8 542.6 337.1 510.5 1.51x 94%
2 8 16 532.7 297.3 642.1 2.16x 121%
4 1 4 1,744.8 1,532.3 1,728.7 1.13x 99%
4 2 8 1,922.9 1,485.0 2,190.4 1.48x 114%
4 4 16 1,862.6 1,356.7 1,962.3 1.45x 105%
4 8 32 1,402.9 1,760.3 1,689.1 0.96x 120%
8 1 8 3,152.6 1,960.6 3,144.0 1.60x 100%
8 2 16 3,157.2 3,413.5 2,839.2 0.83x 90%
8 4 32 2,802.2 2,622.1 2,554.9 0.97x 91%
8 8 64 3,296.0 3,151.1 3,334.9 1.06x 101%

The fix removes the severe one-connection cliff and returns the slowest #917 cases to roughly main-level throughput.

@brentechols brentechols marked this pull request as ready for review July 6, 2026 07:01

@seanmonstar seanmonstar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@seanmonstar seanmonstar force-pushed the sean/bye-bye-big-mutex branch 2 times, most recently from 28d5e03 to 4b63ca9 Compare July 6, 2026 14:59
@seanmonstar

Copy link
Copy Markdown
Member

Welp, I caused a bunch of annoying conflicts, so I just cherry-picked in your commit instead of resolving them in this Github PR, pushed now to original branch/PR.

@seanmonstar seanmonstar closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants