Conversation
Every streaming view item is encoded through `encode_segments` before the framing layer knows whether it will be compressed, and a compressor needs one contiguous input, so `encode_chained` flattens a segmented body back into one buffer. Under the default policy with a client that advertises gzip that is every item past the 16 KiB threshold, and no benchmark covered it. `stream_compressed_encode` drives a real in-process `ConnectRpcService` server-streaming call: a handler yields 16 pre-decoded `OwnedView` items, framed through the batching body the server uses and drained frame by frame, with the views decoded in criterion's setup so the timed region is encode, framing and compression only. Arms pair the generated `encode_segments` against a newtype that takes the contiguous default, with and without gzip negotiated, over one large `string` field and over 1 KiB `string` fields summing to the same size, at 4, 32 and 256 KiB. A pointer check before each arm asserts a frame aliases an item's buffer in exactly the one combination where a capture should reach the body, so a silently flattened segmented arm cannot pass as "no difference". On a dedicated bare-metal box the segmented encode adds at most 1% to the compressed arms, against a 5% resolution floor; the flatten is the payload copy a contiguous encode makes anyway, and what the rope adds around it disappears next to the compressor's cost. The one visible cost is the 1 KiB-fields shape, which pays the rope's doubling tail without capturing anything, and it shows up uncompressed too; that is the gate's problem rather than compression's. So the encode choice is left alone, and the `EncodedStream` doc now bounds the waste it already described instead of implying it is worth avoiding. 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
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.
#271 encodes every streaming view item through
Encodable::encode_segmentsbefore the framing layer knows whether the item will be compressed, andEnvelopeEncoder::encode_chainedflattens a segmented body back to one buffer for the compressor. With the default policy and a client that advertises gzip, every item past the 16 KiB segment threshold pays the rope for nothing. #277 asks how much that costs, and to stop taking the segmented path under compression if the answer is "enough".This adds
stream_compressed_encodetobenches/rpc. Each arm drives a real in-processConnectRpcServiceserver-streaming call whose handler yields 16 pre-decodedOwnedView<BloatEchoView>items, framed through the sameBatchingEnvelopeStreamthe server uses and drained frame by frame; the views are decoded in criterion's setup so the timed region is encode, framing and compression only. The arms differ in whether the item encodes throughencode_segments(segmented) orencode(a newtype leavingencode_segmentsat its contiguous default,contiguous), and in whether the request negotiated gzip. Two shapes: one largestringfield the rope captures whole, and 1 KiBstrings summing to the same size, none capturable (the #278 shape). 4 KiB sits under the segment threshold as a control. A pointer check asserts that a frame aliases an item's backing buffer in exactly the one combination where a capture should reach the body, so a silently flattened segmented arm cannot pass as "no difference".Measured on a dedicated
c7i.metal-24xl(turbo off,performancegovernor, pinned core, 10 s measurement). Median per item, segmented vs contiguous:The threshold for "costs more" was 5%, the run-to-run floor this setup resolves; the 4 KiB rows, where both arms take the identical path, agree within 1.1%. No compressed arm clears it: the segmented encode adds at most 1.0% (about 16 µs per 256 KiB item) to a compressor that costs 1.6 ms for the same item. For the shape whose field the rope captures, the flatten is one payload copy the contiguous encode paid anyway, and the rope's bookkeeping is below what the pair can resolve. The only visible cost is the 1 KiB-fields shape, and it appears without compression too (+47–48%): that is the rope's doubling tail capturing nothing — #278's gate problem, not a compression problem — and #278's fix takes it out of both columns.
So no change to the encode choice. Threading the negotiated encoding to the dispatcher's encode closure without touching the
Dispatchertrait or generated code would need an ambient hint set around the framing layer's poll, and the measurement does not justify a hidden coupling for a sub-1% saving. TheEncodedStreamdoc, which said the segmented encode under compression "was then wasted work", now bounds how much.Fixes #277
No changelog fragment: a benchmark and a doc sentence are not user-visible.
Deferred review findings
clippy::cast_possible_truncationunder pedantic (not enabled); both are provably in range.Gates: nightly fmt
--check; clippy 1.95.0-D warningsclean;cargo test --workspace936 passed;cargo test -p connectrpc --no-default-features513 passed; rustdoc-Dwarningsclean.