perf(uring): gather held buffers into one Mode A forward write (+30%) - #421
Conversation
a0b6658 to
1d68a54
Compare
|
Validation complete on io_uring hardware, rebased onto the merged #418 ( Correctness
100 reps rather than 60 because #415's last hang surfaced at 3/100, and this change touches the bid lifecycle — the invariant those five bugs were about. The 15 repeated full-suite runs cover parallel contention, which single-test repetition structurally cannot reach (that is how the RSS bug in #419 escaped a rack gate and was caught by GitHub CI). PerformanceFive interleaved reps per arm, two guests, 2 workers so the proxy is the bottleneck:
+30.4%, ranges entirely non-overlapping (10.95–12.03 vs 14.41–15.46), and instructions per byte 1.236 → 0.906. The instructions/byte figure is the one that matters for believing this: it is what falsified the direct-forward attempt (1.236 → 1.243, no change, despite a 3% throughput wobble that could have been read as a win). Ready for review from my side. |
6dc2f47 to
61caabb
Compare
+30% on the forwarding path, and instructions per byte from 1.236 to 0.906 — past `run_direct_echo`'s 0.985, within 5% of the ~0.86 kernel floor. Mode A wrote one held buffer per write; direct echo has coalesced a drain's worth into one send since ringline-rs#397. ringline-rs#416 measured what that costs and this change removes it: a forward write now takes up to 16 held buffers as one `sendmsg` (sockets, keeping MSG_WAITALL, which a bare writev would lose) or `writev` (files). Ordering is unchanged — iovecs in hold order, still one write in flight per connection. Getting here cost a NO-GO worth recording. The first theory was that Mode A's cost was the task wake it does per buffer, which direct echo avoids by submitting from the CQE handler. Moving the forward state into the driver and submitting from the completion handler is correct (proxy gate 0/60) and was ~3% *slower*, with instructions/byte unchanged at 1.243. That eliminated the scheduler round-trip and left the completion count as the only remaining explanation — which gathering then confirmed. The driver-side state that attempt introduced is kept, because a batch cannot be assembled from inside a future that is polled once per buffer. SQE memory must outlive the operation, and here that is three things: the buffers, the iovec array, and the msghdr pointing at it. All live in the driver's ForwardWriteState, and the state is installed *before* pointers into it are taken, since moving it afterwards would leave msg_iov's owner dangling even though the iovec heap block survives. Bid release is batch-wide, on completion, submit failure, and fail_forward_write. A short write rebuilds the iovec array from `written` rather than re-slicing: whole consumed buffers drop out and the first survivor starts mid-buffer, and the rebuild only ever runs between operations. The 16-buffer cap has a reason: a batch's bids all return together, so a larger batch delays them and coarsens the hold-cap throttle — and 16 x 16 KiB is already past the ~100 KB where a socket stops having more queued. Two unit tests asserted the single-buffer shape and now assert the batch, strengthened rather than renumbered: gathered iovec counts, the rebuilt array covering exactly the bytes still owed, a fully-written buffer dropping out of the iovecs, and no bid returning until the batch completes — that last one guards the new way to break "exactly one replenish per bid". This also closes ringline-rs#416's one exception. At equal memory the default now measures 14.60 Gbit/s against 15.13 at 64 KiB and 14.22 at 256 KiB — within 3.5% of best, the same order as its deficit on every echo workload — so `forward_to`'s sizing advice is retired rather than revised. Verified on io_uring hardware: clippy -D warnings clean, full suite green, the ringline-rs#415 proxy regression test 0/60, and three interleaved reps per arm with non-overlapping ranges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDHdYwqsnPN4EkhRBvbDgE
The gathering change swapped Mode A's single-buffer write for a batched sendmsg/writev, and the file-sink half of that — writev at an advancing offset — had only unit tests. The proxy soak that validated the change exercises socket sinks exclusively. Two tests, aimed at what batching actually changed: - 1 MiB through a 4 KiB ring, so ~256 buffers and many gathered batches, with a position-dependent payload rather than a fill byte: a batch written at the wrong offset, iovecs out of order, or a short write resubmitting from the wrong place changes content, not just length. - Forward 100,000 bytes while the client sends 64 more, so the boundary falls mid-buffer inside a batch. Asserts both halves of the split: the file holds exactly `len`, and the overshoot is still readable from the connection afterwards. Both fsync before acking, since the client reads the file as soon as it sees the ack. clippy -D warnings on Linux rejected the first version for two `drop_non_drop` calls: `SinkFd` has no Drop impl, and the borrow it holds is immutable, so `sync_all` coexists with it without one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDHdYwqsnPN4EkhRBvbDgE
61caabb to
a115969
Compare
Gathering moved forward submission out of `ForwardToFuture::poll` and into `handle_forward_write`, so the driver advances a forward on its own once armed. That silently removed a guarantee: before it, dropping the future stopped the relay for free, because nothing popped the hold unless someone polled. After it, a `select!` that loses or a `timeout` that fires leaves the relay running to completion off CQEs — streaming up to `len` bytes into a sink nobody is waiting on, with the connection still in the segmented recv domain, so the caller's next `with_data` parks while its bytes go to the sink. Two comments asserted the opposite, both true when ringline-rs#415 wrote them and falsified by the gather change in this PR: the mio `Drop`'s "on io_uring the writes are driven by polling the future", and the test's own mio-only gating rationale. The test is now ungated and runs on both backends. On io_uring it failed 20/20 before this fix and passes 20/20 after, with the assertion that names the symptom: "no echo: the bytes went to the sink instead of the handler". The in-flight write is deliberately untouched: its backings are being read by the kernel, and returning those bids to the provided ring here would let an arriving packet overwrite a `sendmsg` in progress. Its CQE releases them exactly once, and with the progress cleared `advance_forward` submits no successor — so queued bytes finish and no further bytes are taken from the source, which is the contract mio already documents. Held-but-unwritten bytes go to the accumulator via the same `settle_forward_end` the normal end of a forward uses, so a cancel loses no data the peer already sent. `ForwardProgress` gains an epoch so a future dropped after its own forward resolved cannot settle a later one that armed on the same connection in between — the same generation-tagging discipline `forward_write` already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDHdYwqsnPN4EkhRBvbDgE
…oved Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FDHdYwqsnPN4EkhRBvbDgE
Fourth of four, split out of #417. The runtime change — review this one hardest.
+30% on the forwarding path, instructions per byte 1.236 → 0.906 — past
run_direct_echo's 0.985, within 5% of the ~0.86 kernel floor.Ranges do not overlap. Two X710 guests, 2 workers so the proxy is the bottleneck.
What changed
Mode A wrote one held buffer per write; direct echo has coalesced a drain's worth into one send since #397. A forward write now takes up to 16 held buffers as one
sendmsg(sockets — keepingMSG_WAITALL, which a barewritevwould lose) orwritev(files). Ordering is unchanged: iovecs in hold order, still one write in flight per connection.A NO-GO on the way, kept in the journal
The first theory was that the cost was the task wake per buffer that direct echo avoids. Moving the forward state into the driver and submitting from the completion handler is correct (proxy gate 0/60) and was ~3% slower, with instructions/byte unchanged at 1.243. That eliminated the scheduler round-trip and left the completion count as the only remaining explanation — which gathering then confirmed. The driver-side state that attempt introduced is kept, because a batch cannot be assembled from inside a future polled once per buffer.
Together they decompose the per-completion cost cleanly: scheduler round-trip ≈ 0, completion count ≈ all of it.
The risky parts, and what guards them
SQE memory must outlive the operation, and here that is three things — the buffers, the iovec array, and the
msghdrpointing at it. All live inForwardWriteState, and the state is installed before pointers into it are taken: moving it afterwards would leavemsg_iov's owner dangling even though the iovec heap block survives.Bid release is now batch-wide — on completion, on submit failure, and in
fail_forward_write. This is the new way to break "exactly one replenish per bid", the invariant that cost #415 five bugs, so the tests assert it directly:The 16-buffer cap is not a round number: a batch's bids all return together, so a larger batch delays them and coarsens the hold-cap throttle — and 16 × 16 KiB is already past the ~100 KB where a socket stops having more queued (#416).
It closes #416's one exception
At equal memory the default now measures 14.60 Gbit/s against 15.13 at 64 KiB and 14.22 at 256 KiB — within 3.5% of best, the same order as its deficit on every echo workload. 256 KiB is now worse than the default. So
forward_to's sizing advice is retired rather than revised.Verification
GitHub CI does build and run the io_uring backend (
has_io_uringis set onubuntu-latest), including the unit tests this change rewrites — so the greenchecks are real coverage, not a formality. What it cannot give is repetition
under load on real io_uring hardware, which is what the rack adds.
Authoritative gate, exclusive anvil guest (56 cores, io_uring):
-D warnings, io_uring-D warnings, mioforward_to_connproxy regression testforward_to_file(the two new tests)cargo test --all, io_uringcargo test --all, mio100 reps rather than 60, per this PR's own earlier bar: #415's last hang took
3/100 to surface and this change moves bid release from per-write to per-batch.
The
spawn_with_handle_abortfailure seen in an early gate is not this PR.A/B against
b0e0605across three environments — 100 baseline full-suite runsand 100 branch runs — produced no forward-path or file-sink failure on either
arm; the only failures were pre-existing flakes (
spawn_with_handle_abort,and two connect-path tests) at equal rates on both sides.
A regression this PR introduced, found in review and fixed here
Gathering silently voided drop-cancellation on io_uring. Submission moved
from
ForwardToFuture::pollintohandle_forward_write, so the driver nowadvances a forward on its own. Before that, dropping the future stopped the
relay for free. After it, a
select!that loses or atimeoutthat firesleaves bytes streaming to a sink nobody awaits, with the connection still in
the segmented recv domain — so the caller's next
with_dataparks while itsdata goes to the sink.
Nothing caught it: the only test covering a dropped forward was
#[cfg(not(has_io_uring))], gated off on the premise this change falsified("on io_uring the writes are driven by polling the future, so dropping it
stops them by itself"). The mio
Dropdoc carried the same stale claim.Ungated, the test failed 20/20 on io_uring with
no echo: the bytes went to the sink instead of the handler, and passes20/20 with the fix.
The fix mirrors mio's, plus one thing the index-keyed driver state forces:
sendmsgthe kernel is still reading; returning those bids to the providedring in
Dropwould let an arriving packet overwrite a send in progress.Its CQE releases them exactly once, and with the progress cleared
advance_forwardsubmits no successor — landing on the contract mio alreadydocuments: what is queued completes, no further bytes are taken.
settle_forward_endthe normal end of a forward uses, so a cancel loses nodata the peer already sent.
ForwardProgressgains an epoch, so a future dropped after its ownforward resolved cannot cancel a later one that armed on the same
connection — the generation-tagging discipline
forward_writealready uses.Both stale comments are corrected, and the journal entry records the general
lesson: moving work from a future into a completion handler moves its
cancellation semantics too, and a cfg-gate on a test is an assertion about
behaviour that gets quieter as it gets wronger.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FDHdYwqsnPN4EkhRBvbDgE