server: encode view responses without copying their large fields - #232
Merged
Conversation
`Encodable::encode` returns one contiguous `Bytes`, which is where a large payload gets copied: the encoder allocates a buffer the size of the whole message and memcpys the payload into it. buffa 0.9 can avoid that — its `Rope` sink captures a large `bytes::Bytes` field as a reference-counted segment instead — but a rope's segments cannot survive a signature that returns a single buffer. This adds the shape without changing any behaviour yet. `Encodable::encode_segments` is a provided method defaulting to today's contiguous result, so every existing implementation keeps working untouched, and `EncodedBody` carries either one buffer or several. The single-buffer case stays unboxed, so a small message that was never worth segmenting costs no allocation to carry. Only the owned-message proto path overrides it so far. Nothing calls it yet — wiring it through the dispatcher and envelope, where the segments become separate body frames, is the next step. The test that matters is the invariant one: however the encoder divides the output, concatenating it reproduces exactly what the contiguous encode produced. A divergence there would be a wire-format bug rather than a performance regression, so it is pinned across a range of segment thresholds. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
Adds `encode_view_body_segments`, the encoder that captures a view's large borrowed fields by reference count instead of copying them, plus the benchmark that decided its shape. Still nothing calls it; wiring the segments out to the socket comes next. The benchmark is the point of this commit. Encoding a four-string message through a rope, against the contiguous encode it would replace: field size contiguous rope+backing 256 B 125 ns 129 ns 1 KiB 141 ns 137 ns 16 KiB 1.49 us 432 ns 256 KiB 36.4 us 442 ns 1 MiB 249 us 434 ns Flat above the threshold, because only the framing is being written. Two findings changed the design. Owned messages are deliberately left on the contiguous path: their `string` and `bytes` fields are `String` and `Vec<u8>` under the default codegen mapping and cannot be handed over by reference, so a rope captures nothing and only adds cost — measured at 30.6us -> 43.5us for 256 KiB. Only views borrow their fields out of a buffer a rope can capture from, and they do so whatever bytes mapping the message was generated with, so this is the path that pays and it needs no configuration from the user. The segment threshold is the framing layer's, not buffa's default. At 4 KiB a message can clear the gate while none of its fields do, so the rope captures nothing and the encode goes 141ns -> 365ns; matching the framing threshold keeps small messages at parity and makes each segment map to one body frame. A rope pointed at the wrong buffer captures nothing but must still encode correctly, so that case is pinned too, alongside the invariant that concatenating the segments reproduces the contiguous encoding exactly. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
Completes the path the previous commits prepared. `EncodedResponse` now carries an `EncodedBody` rather than a `Bytes`, generated `OwnedView` bodies encode through a rope backed by the buffer they were decoded from, and the gRPC unary body emits one frame per segment instead of one payload. A view response with large borrowed fields now reaches the socket without those fields ever being copied. Wire bytes are identical — only HTTP frame boundaries move, and envelope framing has never depended on those. Server conformance is 3600 passed, 0 failed. Where segments cannot help, the code says so and flattens: - compression reads every byte and emits one buffer, so it ends segmentation by construction; - Connect unary has no envelope, so there is no frame boundary to hang a segment on; - the streaming path re-envelopes per item downstream; - interceptors inspect and replace whole bodies, so `Payload` stays contiguous — which is what keeps this off interceptor authors. `EncodedResponse` is public through the `Dispatcher` trait, so widening it is a breaking change for anyone implementing custom dispatch: `Bytes` converts with `.into()`, and `.into_contiguous()` recovers a single buffer. That is the whole migration, and it is confined to dispatch — the ~10 call sites it touched here were all construction or assertion of exactly that shape. The codegen override goes on the `OwnedView` impl only. A bare view has borrows but no buffer to name, and a rope with nothing to capture from is slower than a contiguous encode, so that impl keeps the default. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
Review of the previous commit found four ways the segmented path could lose, none of which the conformance suite or the unit tests would catch because all four produce correct bytes. A response that captures from its request's buffer keeps that whole buffer alive until it finishes flushing. Answering a 64 MiB upload with a 32 KiB summary therefore held 64 MiB per in-flight response where it used to hold 32 KiB, trading a copy for memory proportional to the request rather than the reply. The encoder now copies unless the response is most of the buffer it borrows from, which is the case where the buffer was staying alive anyway. The size gate compared the whole message against a per-field threshold, so a message of many medium fields cleared it and captured nothing — landing everything in a rope tail that grows by doubling instead of one sized allocation. The gate now also requires the ratio above; buffa 0.9 exposes no way to pre-size the tail, so the remaining case is documented where it bites rather than fixed. A rope flushes its tail before each capture, so a multi-field view came out as alternating tag fragments and payloads, and every fragment became its own HTTP frame — three bytes behind a nine-byte frame header. Segments below the threshold are now merged into their neighbour, which copies the fragment and leaves the payloads untouched. `MaybeBorrowed` did not forward `encode_segments`. It is the wrapper the docs point at for avoiding a copy, so it was silently undoing the reason a handler picked it. Also: `encode_view_body` returned a 2 GiB response as a panic where its segmented sibling returned an error, and both now share one checked size helper; `encode_parts` collapses into `encode_body_parts`, which it was a special case of; and `encode_body_parts` gains the test it should have had, sweeping every branch to assert the header declares exactly the bytes that follow it. The changelog said interceptors were unaffected, which was true of migration and misleading about speed — a call through an interceptor takes the contiguous path, and now says so. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
iainmcgin
force-pushed
the
iain/buffa-0.9-vectored
branch
from
July 20, 2026 19:32
476e414 to
8d16552
Compare
iainmcgin
marked this pull request as ready for review
July 20, 2026 19:32
The Clippy job pins stable 1.95, which rejects the `&chained` in the refcount envelope test as a needless borrow: the slice pattern already binds a reference. `json_encoding_stays_contiguous` called the JSON codec ungated, so it panicked under `cargo test -p connectrpc --no-default-features`, where the codec returns Unimplemented. Every other JSON success-path test in the file is already gated the same way, and `json` is a default feature, so the test still runs in the normal suite. CONTRIBUTING listed the minimal-features job as `cargo check` alone. It also runs `cargo test`, which is why a mis-gated test fails there, so say so. Also drop a benchmark name that no longer resolves (`view_encode` is the criterion group; `view_rope_encode` is the bench target), state the capture threshold as the half the predicate actually tests rather than "most of", and remove an argument for future work from a comment that should only describe the code. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
iainmcgin
enabled auto-merge
July 20, 2026 20:11
azdagron
approved these changes
Jul 20, 2026
EffortlessSteven
pushed a commit
to EffortlessSteven/connect-rust
that referenced
this pull request
Aug 3, 2026
## Summary `coalesce_small`, added in connectrpc#232, merges only *runs* of consecutive sub-threshold segments. Its name, its doc — "Merge segments below `min_segment` into their neighbour" — and its test's name all claimed it merges an isolated fragment into an adjacent one. It does not. The shape that matters is the one the doc itself describes: a rope flushes its pending tail before each capture, so a view with several large fields yields alternating tag/length fragments and captured payloads. In that shape no fragment has a small neighbour, so nothing merges. The test made this concrete and then mislabelled it — `tiny_segments_are_merged_into_their_neighbours` asserted `merged.len() == 5` on a five-element alternating input, which is the assertion that nothing merged. ## The behaviour is correct and unchanged A `Bytes` is an immutable `{ptr, len, refcount}` view with no spare capacity and no interior mutability. Every zero-copy operation it offers narrows the window; none widens it. `BytesMut::unsplit` can rejoin without copying, but only for adjacent slices of one originating buffer, and a freshly built tag/length fragment is not contiguous with a payload captured from the request's backing buffer. So folding a 2-byte fragment onto a neighbouring 32 KiB capture — in either direction — forces a fresh allocation and a full copy of that capture, destroying the refcount aliasing this whole path exists to create. Leaving the fragment as its own segment is the right trade, and the residual cost is one 9-byte HTTP/2 frame header per captured field. ## What changes The helper becomes `coalesce_small_runs`. The doc describes runs, and records why an isolated fragment is kept separate — including that the fragment itself is still re-copied into a run of its own, so what survives untouched is the capture, not the fragment. The test is renamed to `isolated_fragments_stay_their_own_segments`. A second test covers the merging branch. In the alternating shape, `pending` never accumulates more than one fragment before it is flushed, so the run-accumulation path the function is named for had no direct coverage: `consecutive_fragments_merge_into_one_segment` drives three adjacent fragments into one segment and pins that the capture following them is not copied. No behaviour change, no public API change — the helper is private. ## Note on connectrpc#232's description connectrpc#232's merged description lists "tiny rope fragments becoming their own frames" among four defects it says are fixed. That claim was wrong; the frames remain, deliberately, for the reason above. The changelog fragment does not repeat the claim, so the release notes are unaffected. ## Testing Clippy on the pinned 1.95 toolchain, `cargo test -p connectrpc --no-default-features`, 56 test suites, lint, and fmt all clean. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
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
2 participants
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.
Summary
Carries buffa 0.9's reference-counted encode segments from the encoder all the way to the HTTP body, so a gRPC or gRPC-Web unary response whose body is an
OwnedViewnever copies its large fields.A view's fields are slices into the buffer the view was decoded from. A rope told about that buffer takes a large field by reference instead of copying it into the encode buffer, and the segments then become their own HTTP data frames. Past the framing threshold the encode stops growing with the payload, because only the framing is still being written.
Measured on a dedicated bare-metal instance (
benches/rpc/view_rope_encode, four-string message, contiguous vs segmented):The control is the part worth reading: an unbacked rope at 1 MiB costs 363 µs, identical to the contiguous encode. The rope machinery is free; the reference-count capture is the entire effect.
Where this deliberately does nothing
Segmenting is skipped wherever it would not pay, each guarded in code rather than left to chance:
stringandbytesfields areStringandVec<u8>, which cannot be handed over by reference. Routing them through a rope only adds cost.Reach, stated plainly
This lands on gRPC and gRPC-Web unary responses. Connect unary still flattens because its body type hardcodes
Full<Bytes>; multiple HTTP/2 DATA frames concatenate to the same Connect body, so that path could carry segments and is worth doing — Connect being the primary protocol. Streaming re-envelopes per item and is the next piece of work. A call with an interceptor configured takes the contiguous path, because an interceptor may replace the whole body.Breaking change, confined to custom dispatch
EncodedResponseis nowResponse<EncodedBody>rather thanResponse<Bytes>. This is reachable through the publicDispatchertrait, so anyone implementing custom dispatch is affected: construct withBytes::into(), and recover a single buffer withEncodedBody::into_contiguous().Interceptor authors need no change —
UnaryResponsestill carries a contiguousPayload. That seam is what keeps the blast radius to dispatch rather than every extension point.Builds on
Both prerequisites are now merged, and this branch is rebased onto
main: #233 (the buffa 0.9 adoption, which supplies the rope andEncodeSink) and #219, whose framing emission path this reuses rather than reimplementing.Testing
Server conformance 3600 passed / 0 failed, 56 test suites, lint, docs, and
task generate:all(no drift) all green — re-run after rebasing ontomain, not carried over from the stacked base.Envelope::encode_body_partshas a table test sweeping empty, sub-threshold contiguous, sub-threshold segmented, over-threshold contiguous, two-segment and many-segment bodies, asserting in each that the envelope header declares exactly the byte count that follows it and that reassembly equals the contiguous envelope.A high-effort review pass found four defects that produced correct bytes and so would never have failed a test — buffer pinning, a gate that compared whole-message size against a per-field threshold, tiny rope fragments becoming their own frames, and
MaybeBorrowedsilently not forwarding the segmented encode. All four are fixed in the final commit, "response: only segment where it pays, and merge the fragments", whose message carries the reasoning.One known limitation left in place: a message that clears the gate while none of its individual fields do lands everything in a rope tail that grows by doubling rather than one sized allocation. buffa 0.9 exposes no way to pre-size the tail, so it is documented where it bites rather than worked around.