Avoid flushing h2 socket while holding stream locks

[brentechols]

Avoid flushing h2 socket while holding stream locks

Related: #531

There is also an active broader draft refactor in #917. This PR is intentionally smaller: it targets the specific path discussed in #531 where Streams::poll_complete can hold stream/send-buffer locks while the codec flush path reaches the socket.

Summary

This change splits HTTP/2 stream draining into two phases so the connection task no longer holds the shared stream-state and send-buffer mutexes while flushing or polling writes on the underlying socket.

Previously, Streams::poll_complete held Inner and SendBuffer locks while Prioritize::poll_complete could call into Codec::poll_ready / Codec::flush, which can reach poll_write_buf on the socket. Under high write concurrency on one HTTP/2 connection, stream producer tasks contend with the connection task while the connection task is doing I/O progress.

The new flow is:

• ensure codec/socket write readiness before taking stream locks
• lock stream state and buffer pending frames only while the codec has local buffer capacity
• release the locks
• flush the codec/socket
• briefly re-lock to reclaim partially/fully written DATA frames and requeue as needed

The patch also adds a targeted write-contention benchmark that sends many response body chunks concurrently over a single HTTP/2 connection. It can be run with:

H2_BENCH=write-contention cargo bench --bench main

The benchmark is configurable:

H2_WRITE_CONTENTION_STREAMS=512
H2_WRITE_CONTENTION_CHUNKS_PER_STREAM=128
H2_WRITE_CONTENTION_CHUNK_SIZE=16384
H2_WRITE_CONTENTION_WORKER_THREADS=4

Benchmark

Run environment:

• WSL Ubuntu 24.04
• Rust 1.96.1 stable
• workload: 512 streams x 128 chunks x 16 KiB = 1 GiB over one HTTP/2 connection

Median elapsed time over 3 alternating runs per worker-thread count:

Worker threads	Baseline median	Patched median	Change
1	336 ms	337 ms	roughly flat
2	358 ms	238 ms	33.5% faster
4	382 ms	337 ms	11.8% faster
8	407 ms	340 ms	16.5% faster

Median throughput:

Worker threads	Baseline	Patched
1	3043 MiB/s	3038 MiB/s
2	2853 MiB/s	4294 MiB/s
4	2679 MiB/s	3037 MiB/s
8	2513 MiB/s	3004 MiB/s

The one-worker case is effectively flat, as expected: there is little cross-thread lock contention. The multi-worker runs show the intended effect.

Testing

cargo fmt --all --check
cargo check
cargo test -p h2-tests reserved_capacity_assigned_in_multi_window_updates -- --exact --nocapture
cargo test -p h2-tests small_data_frames_reclaimed_before_buffering_next -- --exact

The new small_data_frames_reclaimed_before_buffering_next regression test fails on the pre-fix PR commit (3a42547) with the in_flight_data_frame assertion, and passes after reclaiming the DATA frame before buffering another frame.

The write-contention benchmark was run before/after this change in WSL Ubuntu 24.04. On this Windows machine, checking the benchmark target currently stops in the existing tokio-rustls dev-dependency chain because aws-lc-sys needs additional native build tools (cmake/NASM) before it reaches h2 code.

[seanmonstar]

Very cool! Also, a few weeks ago I put a significant rewrite in #917.

(Notably, I don't personally have great multi threaded testing environments.)

[brentechols]

Ya I realized that after working on this :) maybe not needed then? I'll leave it up to you - I'm still reading through the PR to understand how everything is changing. Is this still required after the refactor?

[seanmonstar]

Well, I can't guarantee what I did is good, though conceptually it moves more towards queues which tends to help. Some reports in that issue suggest good numbers, now that I've fixed up most bugs in it.

If you have an environment that can stress test any sorts of real world loads, more data is always welcome! (If it turns out what I did is bad, what you did here would be a great step!)

[brentechols]

Let me see what I can do - should be able to take a glance at this at some point this coming week. Really excited to see how the perf looks :)