From ac960fab2ffa24aa50633fe0293d6ff9bbb916c6 Mon Sep 17 00:00:00 2001 From: Iain McGinniss <309153+iainmcgin@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:55:08 -0700 Subject: [PATCH] response: gate the segmented view encode on a capturable field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `worth_segmenting` decided whether a view body goes through a rope from the size of the whole message, but a rope only captures an individual `bytes` or `string` field at or above the 16 KiB framing threshold. A 20 KiB message of twenty 1 KiB fields cleared the gate, took the rope, captured nothing, and wrote every byte into the rope's tail — a `BytesMut` that starts empty and doubles six times for that shape — before the single segment collapsed back to `Contiguous`. On a stream that was paid per item, and a chunked log or metrics export is exactly this shape. The message size is now only the first gate. A message that passes it is walked once more by `CaptureProbe`, an `EncodeSink` that writes nothing and records whether a rope with the same backing buffer and threshold would capture at least one field, using the rope's own rule. Only then is the rope built; everything else goes straight to one sized buffer, including a view re-encoded against the wrong backing buffer. The `size * 2 >= backing_len` memory-pinning rule is unchanged. The probe costs one no-copy walk plus a `compute_size` rerun, because `write_to` consumes the `SizeCache` cursor and buffa 0.9 cannot rewind it; both are proportional to the field count, and a message below the threshold pays neither. The `view_rope_encode` benchmark gains a `view_encode_shapes` group that holds the payload size roughly fixed and varies how it divides into fields, so the gate's decision and the probe's cost are both readable against the contiguous and rope floors. Signed-off-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com> --- .../unreleased/Fixed-20260825-203239.yaml | 18 + benches/rpc/README.md | 2 +- benches/rpc/benches/view_rope_encode.rs | 114 +++++- connectrpc/src/response.rs | 335 ++++++++++++++++-- 4 files changed, 438 insertions(+), 31 deletions(-) create mode 100644 .changes/unreleased/Fixed-20260825-203239.yaml diff --git a/.changes/unreleased/Fixed-20260825-203239.yaml b/.changes/unreleased/Fixed-20260825-203239.yaml new file mode 100644 index 00000000..7dfd2e5e --- /dev/null +++ b/.changes/unreleased/Fixed-20260825-203239.yaml @@ -0,0 +1,18 @@ +kind: Fixed +body: |- + **A view response whose fields are all small no longer pays for a rope + that captures nothing** ([#278]). The segmented encode was gated on the + size of the whole message, but a rope only captures an individual `bytes` + or `string` field at or above the 16 KiB framing threshold, so a message + made of many small fields — a chunk of log records, a metrics batch — took + the rope, captured nothing, and copied itself into a doubling tail before + collapsing back to one contiguous buffer, once per item on a stream. The + gate now also walks the view's fields without copying a byte and takes the + rope only when at least one field would be captured; everything else goes + straight to a single sized buffer. The walk costs a pass over the fields + and a second size computation, proportional to the field count rather + than the payload, and a message below the threshold pays neither. The + encoded bytes are unchanged. + + [#278]: https://github.com/connectrpc/connect-rust/issues/278 +time: 2026-08-25T20:32:39.333194520+00:00 diff --git a/benches/rpc/README.md b/benches/rpc/README.md index bdb99e39..e59bb66d 100644 --- a/benches/rpc/README.md +++ b/benches/rpc/README.md @@ -9,7 +9,7 @@ Benchmark crate for `connectrpc`. `publish = false`; nothing here ships. | `rpc_bench` | Full-stack unary/stream RPC over loopback HTTP, Connect/gRPC/gRPC-Web × proto/JSON. Needs a running server (`cargo run --bin echo_server` etc.). | `cargo bench --bench rpc_bench` | | `cross_impl_bench` | Cross-implementation comparison vs tonic. | `cargo bench --bench cross_impl_bench` | | `echo_bloat` | Codec-layer (no HTTP) `{owned,view}×{decode,encode}` sweep across five payload shapes + a 1→N fanout sweep. Motivates the future view-response handler API. | `cargo bench --bench echo_bloat` | -| `view_rope_encode` | Encode cost of a response view, contiguous vs a rope backed by the view's own buffer, swept either side of the segment threshold. Shows what the segmented response path buys and what it costs below the threshold. | `cargo bench --bench view_rope_encode` | +| `view_rope_encode` | Encode cost of a response view, contiguous vs a rope backed by the view's own buffer, swept either side of the segment threshold, and across field shapes the size alone cannot tell apart (many small fields, one large field among them). Shows what the segmented response path buys, what it costs below the threshold, and what its field probe costs. | `cargo bench --bench view_rope_encode` | Filter by criterion regex: `cargo bench --bench echo_bloat -- fanout` or `-- map_dominated`. diff --git a/benches/rpc/benches/view_rope_encode.rs b/benches/rpc/benches/view_rope_encode.rs index 825ca383..282dfb75 100644 --- a/benches/rpc/benches/view_rope_encode.rs +++ b/benches/rpc/benches/view_rope_encode.rs @@ -1,4 +1,5 @@ -//! What encoding a response view through a rope costs, across payload sizes. +//! What encoding a response view through a rope costs, across payload sizes +//! and across field shapes the payload size cannot tell apart. //! //! Encoding a message into one contiguous buffer copies every field into it. //! A view's fields are slices into the buffer the view was decoded from, so a @@ -9,8 +10,9 @@ //! //! - `contiguous` — the copy-everything baseline. //! - `rope_backed` — the rope with the view's own buffer, so captures engage. -//! - `encode_view_segments` — the production entry point, including its size -//! gate, which is what makes the small sizes match the baseline. +//! - `encode_view_segments` — the production entry point, including its +//! gates: the size rule, which is what makes the small sizes match the +//! baseline, and the field probe that `view_encode_shapes` exercises. //! - `owned_contiguous` — an owned message, whose `String` fields cannot be //! captured; this is why owned bodies are left on the contiguous path. //! - `rope_unbacked` — a rope with no buffer to capture from. Separates the @@ -18,14 +20,19 @@ //! //! Above the segment threshold the encode is payload-independent: only the //! framing is still being written. +//! +//! `view_encode_shapes` holds the payload size roughly fixed and varies how it +//! divides into fields, because the production gate has to decide from the +//! fields — a message of many small ones must not pay for a rope, one large +//! field among them must — and the probe that decides it walks every field. use buffa::view::MessageView; use buffa::{Rope, ViewEncode}; use bytes::Bytes; use connectrpc::{CodecFormat, Encodable}; use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; -use rpc_bench::proto::bench::v1::__buffa::view::FewLargeStringsView; -use rpc_bench::proto::bench::v1::FewLargeStrings; +use rpc_bench::proto::bench::v1::__buffa::view::{BloatEchoView, FewLargeStringsView}; +use rpc_bench::proto::bench::v1::{BloatEcho, FewLargeStrings}; /// Build a `FewLargeStrings` whose four string fields are `each` bytes, then /// return its encoded wire bytes. The view decoded from these borrows each @@ -125,5 +132,100 @@ fn bench_view_encode(c: &mut Criterion) { group.finish(); } -criterion_group!(benches, bench_view_encode); +/// Wire bytes for a `BloatEcho` carrying `count` tags of `each` bytes and a +/// `user_agent` of `blob` bytes: with `blob = 0` a message that can clear the +/// segment threshold as a whole while no single field comes near it, and with +/// a large `blob` the same small fields around one capturable one. +fn encoded_many_small(count: usize, each: usize, blob: usize) -> Bytes { + let msg = BloatEcho { + tags: vec!["t".repeat(each); count], + user_agent: "u".repeat(blob), + ..Default::default() + }; + Bytes::from(buffa::Message::encode_to_vec(&msg)) +} + +/// Wire bytes for a `FewLargeStrings` with one `large` field and three +/// `small` ones: the shape where exactly one field earns its own segment. +fn encoded_one_large(large: usize, small: usize) -> Bytes { + let msg = FewLargeStrings { + body_a: "x".repeat(large), + body_b: "x".repeat(small), + body_c: "x".repeat(small), + body_d: "x".repeat(small), + ts: 1, + seq: 2, + ..Default::default() + }; + Bytes::from(buffa::Message::encode_to_vec(&msg)) +} + +/// The production entry point against field shapes the size gate alone +/// cannot tell apart. A ~20 KiB message of small fields must take the +/// contiguous path, whether it is 20 fields of 1 KiB or 500 of 40 B; one +/// capturable field among them must take the rope. `contiguous` is the floor +/// for the first kind and `rope_backed` for the second, and the gap between +/// `encode_view_segments` and its floor is the probe's cost, which grows with +/// the field count — the 500-field shapes are there to show it. +fn bench_view_encode_shapes(c: &mut Criterion) { + let mut group = c.benchmark_group("view_encode_shapes"); + + let many_small = encoded_many_small(20, 1024, 0); + let one_large = encoded_one_large(64 * 1024, 1024); + let dense_small = encoded_many_small(500, 40, 0); + let dense_one_large = encoded_many_small(500, 40, 64 * 1024); + + macro_rules! shape { + ($name:literal, $buffer:expr, $view_ty:ty) => {{ + let buffer: &Bytes = $buffer; + let view = <$view_ty>::decode_view(buffer).expect("decode view"); + group.throughput(Throughput::Bytes(buffer.len() as u64)); + + group.bench_with_input(BenchmarkId::new("contiguous", $name), &view, |b, view| { + b.iter(|| std::hint::black_box(view.encode_to_bytes())) + }); + + // Pinned to the framing threshold rather than buffa's 4 KiB + // default, so this floor is the rope the production path builds. + group.bench_with_input(BenchmarkId::new("rope_backed", $name), &view, |b, view| { + b.iter(|| { + let mut rope = Rope::with_min_segment(16 * 1024).with_backing(buffer.clone()); + ViewEncode::encode(view, &mut rope); + std::hint::black_box(rope.into_segments()) + }); + }); + + group.bench_with_input( + BenchmarkId::new("encode_view_segments", $name), + &view, + |b, view| { + b.iter(|| { + std::hint::black_box( + connectrpc::__codegen::encode_view_body_with_min_segment( + view, + buffer, + CodecFormat::Proto, + 16 * 1024, + ) + .expect("encode"), + ) + }); + }, + ); + }}; + } + + shape!("many_small_20x1KiB", &many_small, BloatEchoView); + shape!("one_large_64KiB+3x1KiB", &one_large, FewLargeStringsView); + shape!("dense_small_500x40B", &dense_small, BloatEchoView); + shape!( + "dense_one_large_500x40B+64KiB", + &dense_one_large, + BloatEchoView + ); + + group.finish(); +} + +criterion_group!(benches, bench_view_encode, bench_view_encode_shapes); criterion_main!(benches); diff --git a/connectrpc/src/response.rs b/connectrpc/src/response.rs index 3b3bbf5f..3efbcb0e 100644 --- a/connectrpc/src/response.rs +++ b/connectrpc/src/response.rs @@ -690,10 +690,13 @@ pub trait Encodable { /// How large a payload has to be before it earns its own segment is the /// framing layer's decision, not the implementation's — anything smaller /// is copied into the framing buffer downstream regardless, so a smaller - /// threshold spends effort without saving a copy. Implementations that - /// need to encode a view should call + /// threshold spends effort without saving a copy. The threshold applies + /// per field, not to the message: a body whose fields each fall below it + /// should return the contiguous default however large the message is, + /// because a rope there captures nothing. Implementations that need to + /// encode a view should call /// [`__codegen::encode_view_body_segments`](crate::__codegen::encode_view_body_segments), - /// which applies that threshold for them. + /// which applies both rules for them. /// /// # Errors /// @@ -753,7 +756,7 @@ pub fn encode_view_body<'a, V: ViewEncode<'a>>( } } -/// Whether a response of `size` bytes should be encoded through a rope, given +/// Whether a response of `size` bytes may be encoded through a rope, given /// that its captures would alias a `backing` buffer of `backing_len` bytes. /// /// Two ways a rope loses. A response below one segment has nothing large @@ -762,12 +765,108 @@ pub fn encode_view_body<'a, V: ViewEncode<'a>>( /// keeping the whole allocation alive until the response finishes flushing — /// a handler that answers a 64 MiB upload with a 32 KiB summary would hold /// 64 MiB per in-flight response where it used to hold 32 KiB. Copying is -/// cheaper than that. When the response is at least half the buffer it borrows from, -/// the buffer was going to stay alive anyway and the capture is free. +/// cheaper than that. When the response is at least half the buffer it borrows +/// from, the buffer was going to stay alive anyway and the capture is free. +/// +/// Passing here is necessary, not sufficient: the size says nothing about how +/// the bytes divide into fields, which is what [`has_capturable_field`] asks. fn worth_segmenting(size: usize, backing_len: usize, min_segment: usize) -> bool { size >= min_segment && size.saturating_mul(2) >= backing_len } +/// An [`EncodeSink`](buffa::EncodeSink) that writes nothing and records +/// whether a [`Rope`](buffa::Rope) with the same `backing` and `min_segment` +/// would capture at least one field. +/// +/// The rule is the rope's: a slice counts when it is at least `min_segment` +/// long and lies inside `backing`, and an owned +/// [`put_shared`](buffa::EncodeSink::put_shared) segment counts on length +/// alone, since the rope takes those wherever they came from. (Generated views +/// hold `bytes` fields as `&[u8]`, which reach `put_slice`; only a custom +/// shared representation reaches `put_shared`.) The verdict is monotone, so +/// once a capture is found the remaining fields cost one branch each. +struct CaptureProbe<'b> { + backing: &'b Bytes, + min_segment: usize, + found: bool, +} + +impl<'b> CaptureProbe<'b> { + fn new(backing: &'b Bytes, min_segment: usize) -> Self { + Self { + backing, + // The rope's clamp: with `min_segment >= 1` an empty slice can + // never qualify, so its dangling pointer never reaches the + // containment check. + min_segment: min_segment.max(1), + found: false, + } + } + + const fn found_capture(&self) -> bool { + self.found + } +} + +impl buffa::EncodeSink for CaptureProbe<'_> { + // The rope is segmented, so buffa's encoders must take the same branch + // here as they will there — a `bytes::Bytes` field reaches `put_shared` + // only when the sink says it is segmented. + const IS_SEGMENTED: bool = true; + + fn put_u8(&mut self, _value: u8) {} + + fn put_slice(&mut self, src: &[u8]) { + if self.found || src.len() < self.min_segment { + return; + } + let backing_start = self.backing.as_ptr() as usize; + let slice_start = src.as_ptr() as usize; + let ends = ( + backing_start.checked_add(self.backing.len()), + slice_start.checked_add(src.len()), + ); + if let (Some(backing_end), Some(slice_end)) = ends + && slice_start >= backing_start + && slice_end <= backing_end + { + self.found = true; + } + } + + fn put_u32_le(&mut self, _value: u32) {} + + fn put_u64_le(&mut self, _value: u64) {} + + fn put_shared(&mut self, bytes: Bytes) { + if bytes.len() >= self.min_segment { + self.found = true; + } + } +} + +/// Whether a rope backed by `backing` would capture at least one of `view`'s +/// fields, leaving `cache` refilled for the encode that follows. +/// +/// Runs the view's `write_to` into a [`CaptureProbe`]: one walk over every +/// field, nested messages included, with no bytes copied, so the cost is +/// proportional to the field count rather than the payload. `write_to` +/// consumes `cache`'s nested-size cursor as it goes, and buffa 0.9 has no way +/// to rewind it, so `compute_size` runs again before returning — the probe's +/// second walk, and the reason the caller asks [`worth_segmenting`] first. +fn has_capturable_field<'a, V: ViewEncode<'a>>( + view: &V, + cache: &mut buffa::SizeCache, + backing: &Bytes, + min_segment: usize, +) -> bool { + let mut probe = CaptureProbe::new(backing, min_segment); + view.write_to(cache, &mut probe); + cache.clear(); + view.compute_size(cache); + probe.found_capture() +} + /// Merge each *run* of consecutive sub-`min_segment` segments into one. /// /// A rope flushes its pending tail before each capture, so a view with several @@ -827,22 +926,34 @@ fn checked_response_size(size: u32) -> Result { /// curve; above the threshold the encode goes flat, because only the framing /// is still being written. /// -/// `backing` must be the buffer this view was decoded from. A rope pointed -/// anywhere else captures nothing and is slower than a contiguous encode — it -/// still produces correct bytes, so the cost of getting this wrong is silent. -/// A caller with no buffer to give should use [`encode_view_body`]. +/// `backing` must be the buffer this view was decoded from. Pointed anywhere +/// else, nothing can be captured: the probe below finds no field inside the +/// buffer and the encode falls back to the contiguous path, having paid for +/// the probe to learn nothing. The bytes are correct either way, so the cost +/// of getting this wrong is silent. A caller with no buffer to give should +/// use [`encode_view_body`]. /// /// The threshold below which a payload is not worth its own segment is the /// framing layer's, applied here so callers cannot pick a worse one: anything -/// smaller is copied into the framing buffer downstream regardless, and a -/// message can clear a smaller gate while none of its individual fields do, -/// which spends the rope's cost and captures nothing. Matching the framing -/// threshold also makes every large segment map to exactly one body frame. +/// smaller is copied into the framing buffer downstream regardless. Matching +/// the framing threshold also makes every large segment map to exactly one +/// body frame. +/// +/// A rope is taken only when it will capture something. A message can clear +/// the threshold while none of its individual fields do — many small records +/// adding up to tens of KiB — and a rope there captures nothing and costs more +/// than the contiguous encode it replaces. So the message size is only the +/// first gate; the second walks the view's fields without copying a byte and +/// asks whether the rope would take any one of them by reference — a slice +/// of `backing` at or above the threshold, or an owned `Bytes` that size from +/// anywhere. /// /// # Errors /// /// [`ErrorCode::Unimplemented`](crate::ErrorCode::Unimplemented) for -/// [`CodecFormat::Json`], as [`encode_view_body`]. +/// [`CodecFormat::Json`], as [`encode_view_body`], and +/// [`ErrorCode::Internal`](crate::ErrorCode::Internal) for a message past +/// the 2 GiB protobuf limit. #[doc(hidden)] pub fn encode_view_body_segments<'a, V: ViewEncode<'a>>( view: &V, @@ -874,18 +985,22 @@ pub fn encode_view_body_with_min_segment<'a, V: ViewEncode<'a>>( let mut cache = buffa::SizeCache::new(); let size = checked_response_size(view.compute_size(&mut cache))?; - if !worth_segmenting(size, backing.len(), min_segment) { + // Cheap gate first: only a message that passes the size rule + // pays for the field walk. + if !worth_segmenting(size, backing.len(), min_segment) + || !has_capturable_field(view, &mut cache, backing, min_segment) + { let mut buf = BytesMut::with_capacity(size); view.write_to(&mut cache, &mut buf); return Ok(EncodedBody::Contiguous(buf.freeze())); } // Known cost: a rope's tail starts empty and grows by doubling, - // and every field too small to capture lands in it. A message that - // clears the gate while none of its fields do therefore copies - // itself roughly twice over instead of once into a sized buffer. - // buffa 0.9 exposes no way to pre-size the tail; until it does, - // that shape pays for a rope that captures nothing. + // and every field too small to capture lands in it. The probe + // guarantees at least one field is captured, so the tail holds at + // most the message minus that field; buffa 0.9 exposes no way to + // pre-size it, so a message that pairs one large field with many + // small ones still copies the small ones roughly twice over. let mut rope = buffa::Rope::with_min_segment(min_segment).with_backing(backing.clone()); view.write_to(&mut cache, &mut rope); Ok(EncodedBody::from_segments(coalesce_small_runs( @@ -1296,7 +1411,7 @@ impl Response { #[cfg(test)] mod tests { use super::*; - use buffa_types::google::protobuf::__buffa::view::StringValueView; + use buffa_types::google::protobuf::__buffa::view::{ListValueView, StringValueView}; use buffa_types::google::protobuf::StringValue; /// The invariant the whole segmented path rests on: however the encoder @@ -1353,6 +1468,173 @@ mod tests { ); } + /// Wire bytes for a `ListValue` of strings, one per entry of `lengths`. + /// Each string is its own field, so the shape of the message — many small + /// fields, or one large one among them — is the shape of `lengths`. + fn encoded_string_list(lengths: &[usize]) -> Bytes { + use buffa_types::google::protobuf::__buffa::oneof::value::Kind; + use buffa_types::google::protobuf::{ListValue, Value}; + + let list = ListValue { + values: lengths + .iter() + .map(|&len| Value { + kind: Some(Kind::StringValue("x".repeat(len))), + ..Default::default() + }) + .collect(), + ..Default::default() + }; + Bytes::from(buffa::Message::encode_to_vec(&list)) + } + + /// The probe's verdict for the `ListValue` encoded in `buffer`, checked + /// against the refill contract: a real `write_to` must still be able to + /// follow the probe on the same cache. + fn probe_list(buffer: &Bytes, backing: &Bytes, min_segment: usize) -> bool { + let view = ListValueView::decode_view(buffer).expect("decode view"); + let mut cache = buffa::SizeCache::new(); + view.compute_size(&mut cache); + let found = has_capturable_field(&view, &mut cache, backing, min_segment); + + let mut buf = BytesMut::new(); + view.write_to(&mut cache, &mut buf); + assert_eq!( + &buf[..], + &buffer[..], + "cache must be refilled for the encode" + ); + found + } + + /// Whether a real rope with the same backing and threshold captures + /// anything from the `ListValue` in `buffer`: a captured field is a + /// segment whose bytes live inside `backing`. + fn rope_captures(buffer: &Bytes, backing: &Bytes, min_segment: usize) -> bool { + let view = ListValueView::decode_view(buffer).expect("decode view"); + let mut rope = buffa::Rope::with_min_segment(min_segment).with_backing(backing.clone()); + ViewEncode::encode(&view, &mut rope); + rope.into_segments().iter().any(|segment| { + let (start, end) = ( + backing.as_ptr() as usize, + backing.as_ptr() as usize + backing.len(), + ); + let at = segment.as_ptr() as usize; + at >= start && at + segment.len() <= end + }) + } + + #[test] + fn many_small_fields_skip_the_rope() { + // Twenty 1 KiB strings add up to 20 KiB, which clears the 16 KiB size + // gate, yet no single field reaches it: a rope here would capture + // nothing and copy the message into a doubling tail. On a stream that + // is paid per item for the life of the stream, and the only symptom is + // a slower encode — the bytes come out right either way. + let min_segment = 16 * 1024; + let buffer = encoded_string_list(&[1024; 20]); + assert!( + worth_segmenting(buffer.len(), buffer.len(), min_segment), + "the size gate alone would send this shape to a rope" + ); + assert!( + !probe_list(&buffer, &buffer, min_segment), + "no field is large enough to capture, so the probe must say so" + ); + + let view = ListValueView::decode_view(&buffer).expect("decode view"); + let body = + encode_view_body_with_min_segment(&view, &buffer, CodecFormat::Proto, min_segment) + .expect("proto encode"); + assert!(matches!(body, EncodedBody::Contiguous(_))); + assert_eq!(body.into_contiguous(), buffer); + } + + #[test] + fn one_large_field_among_small_ones_still_segments() { + // The probe must not over-correct: a single capturable field is worth + // the rope however many small ones surround it, because that one + // field is where the payload's bytes are. + let min_segment = 16 * 1024; + let mut lengths = [1024usize; 20]; + lengths[7] = 64 * 1024; + let buffer = encoded_string_list(&lengths); + assert!(probe_list(&buffer, &buffer, min_segment)); + + let view = ListValueView::decode_view(&buffer).expect("decode view"); + let body = + encode_view_body_with_min_segment(&view, &buffer, CodecFormat::Proto, min_segment) + .expect("proto encode"); + assert!(matches!(body, EncodedBody::Segmented(_))); + assert_eq!(body.into_contiguous(), buffer); + } + + #[test] + fn probe_agrees_with_the_rope() { + // The probe re-implements the rope's capture rule, which lives in + // buffa and is not exported. If the two ever drift the failure is + // silent in both directions — a rope that captures nothing, or a + // contiguous encode where a capture was available — so the probe is + // pinned to the rope's actual behaviour, not to its own rule, across + // the shapes and thresholds the gate has to tell apart. + let shapes: [&[usize]; 5] = [ + &[1024; 20], + &[64 * 1024], + &[1024, 1024, 40 * 1024, 1024], + &[16 * 1024], + &[0, 1, 0], + ]; + let unrelated = Bytes::from(vec![0u8; 128 * 1024]); + for lengths in shapes { + let buffer = encoded_string_list(lengths); + for min_segment in [0usize, 1, 4096, 16 * 1024, 64 * 1024, usize::MAX] { + for backing in [&buffer, &unrelated] { + assert_eq!( + probe_list(&buffer, backing, min_segment), + rope_captures(&buffer, backing, min_segment), + "lengths={lengths:?} min_segment={min_segment} own_backing={}", + std::ptr::eq(backing, &buffer) + ); + } + } + } + } + + #[test] + fn probe_ignores_fields_outside_the_backing_buffer() { + // A rope captures only slices of the buffer it was given; a large + // field that lives elsewhere is copied. The probe must predict the + // rope, not merely size the fields, or a view re-encoded against the + // wrong buffer would take a rope that captures nothing. + use buffa::EncodeSink; + + let backing = Bytes::from(vec![0u8; 64 * 1024]); + let elsewhere = vec![0u8; 32 * 1024]; + let mut probe = CaptureProbe::new(&backing, 16 * 1024); + + probe.put_slice(&elsewhere); + assert!(!probe.found_capture(), "outside the backing buffer"); + probe.put_slice(&backing[..8 * 1024]); + assert!(!probe.found_capture(), "inside, but below the threshold"); + probe.put_slice(&backing[1024..40 * 1024]); + assert!(probe.found_capture()); + + // Owned segments are captured by the rope wherever they came from. + let mut probe = CaptureProbe::new(&backing, 16 * 1024); + probe.put_shared(Bytes::from(elsewhere)); + assert!(probe.found_capture()); + + // A zero threshold is clamped to one, as the rope clamps it, so an + // empty slice — whose dangling pointer could sit anywhere — never + // counts as a capture. + let mut probe = CaptureProbe::new(&backing, 0); + probe.put_slice(&[]); + probe.put_shared(Bytes::new()); + assert!(!probe.found_capture(), "empty slices never capture"); + probe.put_slice(&backing[..1]); + assert!(probe.found_capture(), "one byte clears a clamped threshold"); + } + #[test] fn large_view_fields_are_captured_as_segments() { // The whole point of the exercise: a field larger than one segment, @@ -1452,14 +1734,19 @@ mod tests { #[test] fn view_segments_without_backing_are_still_correct() { - // A rope pointed at the wrong buffer captures nothing, which costs - // speed but must never cost correctness. + // A rope pointed at the wrong buffer captures nothing, which must + // never cost correctness — and since the probe can see that nothing + // lies inside the buffer, it should not cost a rope either. let buffer = encoded_string_value(&"y".repeat(64 * 1024)); let view = StringValueView::decode_view(&buffer).expect("decode view"); let unrelated = Bytes::from_static(b"not the buffer this view came from"); let body = encode_view_body_segments(&view, &unrelated, CodecFormat::Proto).expect("proto encode"); + assert!( + matches!(body, EncodedBody::Contiguous(_)), + "nothing to capture from the wrong buffer, so no rope" + ); assert_eq!( body.into_contiguous(), encode_view_body(&view, CodecFormat::Proto).expect("proto encode")