perf: batch outbound writes with whole-buffer encryption and atomic bundles - #545
Open
carabistouflette wants to merge 5 commits into
Open
Conversation
12 tasks
carabistouflette
force-pushed
the
pr1-batched-outbound-writes
branch
from
August 29, 2026 00:35
84b8cab to
639b3aa
Compare
…undles - StreamEncryptor encrypts whole buffers at once (vanilla CipherBase.encipher model) instead of one async write per byte, retaining partially written encrypted bytes across partial writes and pending polls - OutboundPacket::Bundle makes bundle sends atomic: delimiters and sub-packets are queued as a single message and written under one writer lock, without interleaving other packets - TCPNetworkEncoder::write_packets writes a batch with a single flush; sender tasks drain up to 128 queued packets per lock acquisition via recv_many Measured with criterion on a loopback socket (128 encrypted packets): - batched single flush: 4.59 Melem/s vs 0.57 Melem/s per-packet flush (~8x) - whole-buffer encryption: parity within noise vs per-byte loop on non-backpressuring sinks; the win is one pending-reschedule per buffer instead of per byte under backpressure - regression tests cover bundle atomicity and encryptor buffer retention
carabistouflette
force-pushed
the
pr1-batched-outbound-writes
branch
from
August 29, 2026 10:29
639b3aa to
4b0b9e9
Compare
Batch writes took the encoder by shared reference, so a mid-batch failure left an encoder with mid-packet CFB8 state installed; the queued-disconnect and immediate-write paths would then append to a partially written packet. Hoist write_outbound_batch and OUTBOUND_BATCH_SIZE into player::connection and take the encoder out of the shared slot, restoring it only after the batch and flush succeed. Covers the steel-core and steel-login sender loops.
State the caller contract precisely (continuation retry after a partial write, whole-buffer retry after Pending) and note that poll_flush does not push bytes retained from a partial write.
carabistouflette
marked this pull request as ready for review
August 30, 2026 01:08
11 tasks
kdcokenny
suggested changes
Aug 30, 2026
- StreamEncryptor now uses commit-all buffering (BufWriter semantics): every written buffer is reported fully consumed and its ciphertext is retained until the inner writer accepts it. A sink that accepts partially, or drops bytes past the reported count, can no longer desync the CFB8 stream. poll_flush and poll_shutdown drain retained ciphertext before resolving. - write_outbound_batch bounds the write with OUTBOUND_WRITE_TIMEOUT (30 s): a client that stops reading can no longer wedge the sender task while holding the writer lock. On timeout the encoder is discarded, poisoning the writer slot the same way a failed batch does. Covers the two CHANGES_REQUESTED threads on Steel-Foundation#545: the abc/XY repro reported by kdcokenny now produces abcXY, and a stalled client cannot block kicks or shutdown indefinitely.
6 tasks
Contributor
|
I would like an confirmation that it don't do something similar to with #473 |
|
This pull request has conflicts with the base branch "master". Please resolve those so we can test out your changes. |
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.
Type of change
Description
Overhauls the Java outbound write path. It also adds the criterion benchmarks used for the
measurements below (
steel-protocol/benches/packet_codec.rs: whole-buffer encryptionthroughput, and per-packet-flush vs batched-single-flush socket write patterns), kept in-tree
for future regression tracking:
StreamEncryptorencrypts whole buffers at once (mirroring vanillaCipherBase.encipher) instead of running one asyncpoll_writeper byte, and retains partially written encrypted bytes across polls so a partial socket write can never desync the CFB8 stream.OutboundPacket::Bundlevariant: bundle delimiters and sub-packets are queued as a single channel message, eliminating the multi-producer interleaving race where another packet could land between a bundle's delimiters.TCPNetworkEncoder::write_packetswrites a batch of packets with a single flush; both sender tasks (pre-play and play) drain up to 128 queued packets per writer-lock acquisition viarecv_many.write_outbound_batch/OUTBOUND_BATCH_SIZEare shared between the play and pre-play sender loops inplayer::connection.How this was tested
cargo test -p steel-core --lib connection::java(16 tests, incl. newsend_encoded_bundle_sends_one_atomic_bundle_message),cargo test -p steel-core --lib player::connection(addsfailed_batch_write_discards_the_encoderandsuccessful_batch_write_restores_the_encoderfor the poison-on-failure behavior),cargo test -p steel-protocol --lib(newencrypts_whole_buffers_like_vanilla_cipherandretains_unwritten_encrypted_bytes_across_partial_writes),cargo test -p steel-login,cargo clippy -r --all-targetsclean.Measurements
Run back-to-back on the same idle machine (Linux x86_64 laptop, Intel Core Ultra 7 258V, Rust
nightly, criterion defaults: 3 s warmup, 5 s measurement, 100 samples; loopback TCP with
TCP_NODELAYand a concurrent drain task). Absolute numbers are machine-specific; the ratiosare what matter.
Outbound write pattern, 128 encrypted
CKeepAlivepackets per batch over loopback:master(36156e9): write + flush per packetBatching alone gives ~8.6x more outbound throughput per writer-lock acquisition vs the
mastersend path — one write syscall per batch instead of one per packet. The encryptorchange contributes ~0: the per-packet rows show it is within noise of
master.Whole-buffer CFB8 encryption throughput into a non-backpressuring sink (isolates the encryptor
from the socket):
masterbyte-at-a-timeThroughput parity (AES dominates; ≤ run-to-run noise). The encryption win is elsewhere: one
task reschedule per buffer instead of per byte when the socket backpressures, and no dropped
bytes when the inner writer returns
Ok(0).How to replicate
On this branch (all three arms above except the
masterrow):For the
masterbaseline row, run the same bench against the commit this PR branches from.The bench needs
write_packets, which only exists on this branch, so copy it onto amasterworktree and drop the batched arm:
Screenshots / logs
N/A (no visual surface).
Checklist
Additional notes
pr2-outbound-byte-budgetis stacked on this branch.