The out-parameter added in #1137 removes the per-burst allocation only for the single-frame path, which is the common one and is where the measured -1.43 allocations per message came from. A burst of 2-4 frames still allocates.
Where it goes
Client::send_raw_bytes_burst (src/client/messaging.rs) drives the multi-frame path through futures::future::join_all, which allocates its own storage for the futures and returns a fresh Vec of results. That Vec is then copied into the caller's buffer, so the path pays what it paid before the out-parameter existed, plus one traversal.
Reported by Codex on #1137 and correct: the optimisation does not reach this path.
Why it was not fixed there
encrypt_and_send sends a SendJob down a channel and awaits a oneshot, so the concurrency is what gets every frame of a burst to the sender before any of them completes. That is what lets the Noise layer coalesce them into one transport write (#1119, #1121). Awaiting the futures in sequence would remove the allocation and the batching with it.
What a fix would look like
Split encrypt_and_send into an enqueue half returning the oneshot::Receiver and an await half. The burst then enqueues every job first, holds the receivers in a SmallVec<[_; 4]> (both call sites cap the burst at 4, so that is inline), and awaits them in order. No join_all, no result Vec, ordering preserved because the receivers are awaited in the order the frames were drained.
That is a change to the send path rather than to the burst helper, which is why it was kept out of a PR about allocation counts on the DM round trip.
What to watch when doing it
- A frame whose enqueue fails must still leave the caller's
results aligned with the frames that were drained, or a failure gets reported against the wrong frame.
frames must drain on every exit, including the error path. raw_bytes_burst_drains_input_when_disconnected pins this today.- The single-frame fast path must keep filling the caller's buffer rather than replacing it;
raw_bytes_burst_drains_and_reuses_input_on_happy_paths compares as_ptr() across both calls.
The out-parameter added in #1137 removes the per-burst allocation only for the single-frame path, which is the common one and is where the measured -1.43 allocations per message came from. A burst of 2-4 frames still allocates.
Where it goes
Client::send_raw_bytes_burst(src/client/messaging.rs) drives the multi-frame path throughfutures::future::join_all, which allocates its own storage for the futures and returns a freshVecof results. ThatVecis then copied into the caller's buffer, so the path pays what it paid before the out-parameter existed, plus one traversal.Reported by Codex on #1137 and correct: the optimisation does not reach this path.
Why it was not fixed there
encrypt_and_sendsends aSendJobdown a channel and awaits aoneshot, so the concurrency is what gets every frame of a burst to the sender before any of them completes. That is what lets the Noise layer coalesce them into one transport write (#1119, #1121). Awaiting the futures in sequence would remove the allocation and the batching with it.What a fix would look like
Split
encrypt_and_sendinto an enqueue half returning theoneshot::Receiverand an await half. The burst then enqueues every job first, holds the receivers in aSmallVec<[_; 4]>(both call sites cap the burst at 4, so that is inline), and awaits them in order. Nojoin_all, no resultVec, ordering preserved because the receivers are awaited in the order the frames were drained.That is a change to the send path rather than to the burst helper, which is why it was kept out of a PR about allocation counts on the DM round trip.
What to watch when doing it
resultsaligned with the frames that were drained, or a failure gets reported against the wrong frame.framesmust drain on every exit, including the error path.raw_bytes_burst_drains_input_when_disconnectedpins this today.raw_bytes_burst_drains_and_reuses_input_on_happy_pathscomparesas_ptr()across both calls.