From b8b3b0da85e250bd7bc42958d29b7154f958c81f Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 11:30:44 +0000 Subject: [PATCH 1/7] perf(msgpack-bounds): keep the pre-scan's first 8 nesting levels on the call stack --- src/msgpack_bounds.rs | 104 ++++++++++++++++++++++++++++++++++- src/read_allocation_probe.rs | 40 +++++++++++++- 2 files changed, 140 insertions(+), 4 deletions(-) diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index bb62253..be13094 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -25,7 +25,9 @@ fn reject(reason: impl Into) -> MsgpackStructureError { /// A header-only walk: str/bin/ext payloads are skipped by offset, never read, /// and nothing is decoded. It keeps one `u64` per open non-empty collection, so /// at most `min(max_depth, bytes.len())` of them (8 KiB at a depth of 1024), and -/// allocates nothing proportional to any declared length. +/// allocates nothing proportional to any declared length. The first 8 live on +/// the call stack, so a document that nests no deeper than that (a +/// `ByteStorage` envelope nests 2 deep) allocates nothing at all. /// /// Bytes after the document are neither read nor rejected. To find where the /// document ends, call [`msgpack_document_len`], which runs this same walk. @@ -74,6 +76,56 @@ pub fn check_msgpack_structure( msgpack_document_len(bytes, max_depth).map(|_| ()) } +/// Open collections the walk keeps on the call stack before it spills to the heap. +const INLINE_LEVELS: usize = 8; + +/// Elements still owed per open collection, innermost last; its length is the +/// depth. The outermost [`INLINE_LEVELS`] live inline and deeper ones in +/// `spill`, so a shallow document never allocates. `spill` is non-empty only +/// while every inline slot is in use. +struct OpenCollections { + inline: [u64; INLINE_LEVELS], + inline_len: usize, + spill: Vec, +} + +impl OpenCollections { + fn new() -> Self { + Self { + inline: [0; INLINE_LEVELS], + inline_len: 0, + spill: Vec::new(), + } + } + + fn len(&self) -> usize { + self.inline_len + self.spill.len() + } + + fn last_mut(&mut self) -> Option<&mut u64> { + if let Some(last) = self.spill.last_mut() { + return Some(last); + } + let i = self.inline_len.checked_sub(1)?; + Some(&mut self.inline[i]) + } + + fn push(&mut self, owed: u64) { + if self.inline_len < INLINE_LEVELS { + self.inline[self.inline_len] = owed; + self.inline_len += 1; + } else { + self.spill.push(owed); + } + } + + fn pop(&mut self) { + if self.spill.pop().is_none() { + self.inline_len -= 1; + } + } +} + /// Runs the walk [`check_msgpack_structure`] describes and returns the length, /// in bytes, of the document at the start of `bytes`. /// @@ -117,9 +169,9 @@ pub fn msgpack_document_len( let mut pos = 0usize; let mut pending: u64 = 1; // elements owed across all open collections (the root is one) - let mut open: Vec = Vec::new(); // elements still owed per open collection = depth + let mut open = OpenCollections::new(); while pending > 0 { - while open.last() == Some(&0) { + while open.last_mut() == Some(&mut 0) { open.pop(); } let marker = *bytes @@ -317,6 +369,52 @@ mod tests { } } + #[test] + fn depth_bound_is_exact_where_the_stack_spills_to_the_heap() { + for max_depth in INLINE_LEVELS - 1..=INLINE_LEVELS + 1 { + let too_deep = Err(format!("nests deeper than {max_depth} levels")); + for (header, empty) in LEVELS { + let at = |levels, tail: &[u8]| { + check_msgpack_structure(&nested(header, levels, tail), max_depth) + .map_err(|e| e.to_string()) + }; + assert_eq!(at(max_depth, &[0xc0]), Ok(()), "max_depth {max_depth}"); + assert_eq!( + at(max_depth + 1, &[0xc0]), + too_deep, + "max_depth {max_depth}" + ); + assert_eq!(at(max_depth - 1, &[empty]), Ok(()), "max_depth {max_depth}"); + assert_eq!(at(max_depth, &[empty]), too_deep, "max_depth {max_depth}"); + } + } + } + + #[test] + fn a_sibling_after_a_spilled_subtree_is_measured_from_its_own_parent() { + // [a levels of [..[nil]..], b levels of the same]: depth is 1 + max(a, b). + // An `a` past the inline slots spills the stack to the heap and drains + // it again before `b` starts. + let max_depth = INLINE_LEVELS + 2; + for a in 0..=max_depth { + for b in 0..=max_depth { + let doc = [ + vec![0x92], + nested(&[0x91], a, &[0xc0]), + nested(&[0x91], b, &[0xc0]), + ] + .concat(); + let expected = if a.max(b) < max_depth { + Ok(doc.len()) + } else { + Err(format!("nests deeper than {max_depth} levels")) + }; + let got = msgpack_document_len(&doc, max_depth).map_err(|e| e.to_string()); + assert_eq!(got, expected, "a {a}, b {b}"); + } + } + } + #[test] fn map_pairs_cost_two_slots() { assert_eq!(check(&[0x81, 0xa0, 0xc0]), Ok(())); diff --git a/src/read_allocation_probe.rs b/src/read_allocation_probe.rs index 4bb8504..7516251 100644 --- a/src/read_allocation_probe.rs +++ b/src/read_allocation_probe.rs @@ -18,7 +18,9 @@ //! //! The same count, [`bytes_requested`], also pins the write side: the //! `byte_storage` unit test `store_allocates_only_its_output_and_block` checks -//! that `ByteStorage::store` requests only its output and its LZ4 block. +//! that `ByteStorage::store` requests only its output and its LZ4 block, and +//! `accepted_envelope_prescan_allocates_nothing` below checks that the read's +//! structural pre-scan requests nothing for an envelope a writer emits. //! //! This lives in the crate's unit tests, compiled only under `cfg(test)`, //! because the positive control needs a hook inside `extract_payload` @@ -140,6 +142,42 @@ mod tests { } } + /// The pre-scan `retrieve` runs keeps its nesting stack on the call stack + /// for an envelope, current or legacy, at every payload size, so it + /// requests nothing from the allocator. + #[test] + fn accepted_envelope_prescan_allocates_nothing() { + // `retrieve`'s bound (`extract_payload`). + const MAX_DEPTH: usize = 100; + let storage = ByteStorage::new(None); + for len in [0, 40, 64, 1024, 64 * 1024] { + let data: Vec = (0..len).map(|i| (i * 31 % 251) as u8).collect(); + let envelope = storage.store(&data, None).unwrap(); + let e: crate::StorageEnvelope = rmp_serde::from_slice(&envelope).unwrap(); + // Legacy writers encoded `compressed_data` as an array of ints. + let legacy = + rmp_serde::to_vec(&(&e.compressed_data, e.checksum, e.original_size, &e.format)) + .unwrap(); + for (shape, doc) in [("current", envelope), ("legacy", legacy)] { + assert!( + storage.retrieve(&doc).is_ok(), + "[{shape} {len}] not accepted" + ); + let (result, requested) = + bytes_requested(|| crate::check_msgpack_structure(&doc, MAX_DEPTH)); + assert_eq!(result, Ok(()), "[{shape} {len}]"); + assert_eq!(requested, 0, "[{shape} {len}] pre-scan allocated"); + } + } + // Positive control: past the inline levels the stack spills to the + // heap, and this probe sees it. + let deep = [vec![0x91; 9], vec![0xc0]].concat(); + let (result, requested) = + bytes_requested(|| crate::check_msgpack_structure(&deep, MAX_DEPTH)); + assert_eq!(result, Ok(())); + assert!(requested > 0, "probe missed the spilled nesting stack"); + } + /// The same probe catches an `original_size` buffer allocated and freed /// inside the read path, after the envelope decode. One control at the /// ratio vector's 1,000,001 B covers both vectors, as the spec allows. From 59b395f2971c08eff14ae1209cf46ff187aa1b9f Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 11:40:16 +0000 Subject: [PATCH 2/7] perf(msgpack-bounds): track depth in one counter so the common path is one compare --- src/msgpack_bounds.rs | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index be13094..c2b3cc4 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -79,13 +79,13 @@ pub fn check_msgpack_structure( /// Open collections the walk keeps on the call stack before it spills to the heap. const INLINE_LEVELS: usize = 8; -/// Elements still owed per open collection, innermost last; its length is the +/// Elements still owed per open collection, innermost last; `len` is the /// depth. The outermost [`INLINE_LEVELS`] live inline and deeper ones in -/// `spill`, so a shallow document never allocates. `spill` is non-empty only -/// while every inline slot is in use. +/// `spill`, so a shallow document never allocates. `spill` holds exactly the +/// levels past the inline ones. struct OpenCollections { inline: [u64; INLINE_LEVELS], - inline_len: usize, + len: usize, spill: Vec, } @@ -93,36 +93,37 @@ impl OpenCollections { fn new() -> Self { Self { inline: [0; INLINE_LEVELS], - inline_len: 0, + len: 0, spill: Vec::new(), } } fn len(&self) -> usize { - self.inline_len + self.spill.len() + self.len } fn last_mut(&mut self) -> Option<&mut u64> { - if let Some(last) = self.spill.last_mut() { - return Some(last); + match self.len { + 0 => None, + n @ 1..=INLINE_LEVELS => Some(&mut self.inline[n - 1]), + _ => self.spill.last_mut(), } - let i = self.inline_len.checked_sub(1)?; - Some(&mut self.inline[i]) } fn push(&mut self, owed: u64) { - if self.inline_len < INLINE_LEVELS { - self.inline[self.inline_len] = owed; - self.inline_len += 1; + if self.len < INLINE_LEVELS { + self.inline[self.len] = owed; } else { self.spill.push(owed); } + self.len += 1; } fn pop(&mut self) { - if self.spill.pop().is_none() { - self.inline_len -= 1; + if self.len > INLINE_LEVELS { + self.spill.pop(); } + self.len -= 1; } } From e33b843c1ddc54edff574100c64dd9e0eed45fb3 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 11:50:32 +0000 Subject: [PATCH 3/7] docs(readme): note the pre-scan walks an envelope without allocating; ratchet prescan and retrieve budgets --- README.md | 4 +++- benches/perf_ir_baselines.json | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a5e3cf1..13e5b1a 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,9 @@ Malicious payloads claiming `original_size: 500GB` with 100 bytes of data are re envelope bytes **before** MessagePack decoding: nesting deeper than 100 levels, headers declaring more elements or bytes than the input can back, the reserved marker `0xc1` and truncated input are all rejected before decoding, without -allocating in proportion to any declared length. A rejection is +allocating in proportion to any declared length. A document nesting at most 8 +levels deep, which includes every envelope, is walked with no allocation at +all. A rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The same walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted diff --git a/benches/perf_ir_baselines.json b/benches/perf_ir_baselines.json index ef18ef0..1b01db8 100644 --- a/benches/perf_ir_baselines.json +++ b/benches/perf_ir_baselines.json @@ -15,13 +15,13 @@ "keyring_decrypt/1024": 34739, "keyring_decrypt/64": 31754, "keyring_decrypt/65536": 264387, - "prescan/1024": 985, - "prescan/64": 883, - "prescan/65536": 935, - "retrieve/1024": 7323, + "prescan/1024": 910, + "prescan/64": 815, + "prescan/65536": 864, + "retrieve/1024": 7248, "retrieve/1048576": 6676538, - "retrieve/40": 2560, - "retrieve/64": 2552, + "retrieve/40": 2486, + "retrieve/64": 2484, "retrieve/65536": 391697, "store/1024": 22934, "store/1048576": 11912874, From 577793dae8aeb2f9c55058dcff36a3be2df07d38 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 12:02:30 +0000 Subject: [PATCH 4/7] docs(msgpack-bounds): limit the no-allocation claim to envelopes store writes --- README.md | 6 ++++-- src/msgpack_bounds.rs | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 13e5b1a..b478b5c 100644 --- a/README.md +++ b/README.md @@ -259,8 +259,10 @@ envelope bytes **before** MessagePack decoding: nesting deeper than 100 levels, headers declaring more elements or bytes than the input can back, the reserved marker `0xc1` and truncated input are all rejected before decoding, without allocating in proportion to any declared length. A document nesting at most 8 -levels deep, which includes every envelope, is walked with no allocation at -all. A rejection is +levels deep is walked with no allocation at all. That covers every envelope +`store()` writes, which nests 2 deep; a hand-built envelope can nest deeper in a +field the decoder ignores, and its walk then allocates for the levels past 8. +A rejection is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The same walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index c2b3cc4..c257801 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -26,8 +26,8 @@ fn reject(reason: impl Into) -> MsgpackStructureError { /// and nothing is decoded. It keeps one `u64` per open non-empty collection, so /// at most `min(max_depth, bytes.len())` of them (8 KiB at a depth of 1024), and /// allocates nothing proportional to any declared length. The first 8 live on -/// the call stack, so a document that nests no deeper than that (a -/// `ByteStorage` envelope nests 2 deep) allocates nothing at all. +/// the call stack, so a document that nests no deeper than that (an envelope +/// `ByteStorage::store` writes nests 2 deep) allocates nothing at all. /// /// Bytes after the document are neither read nor rejected. To find where the /// document ends, call [`msgpack_document_len`], which runs this same walk. From fbec3f96afd92b84b3c2953c3ea7b3aefcf5b0a0 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 13:59:05 +0000 Subject: [PATCH 5/7] test(read-allocation-probe): drop the reject read's nonzero-allocation check The pre-scan now keeps a shallow envelope's nesting on the call stack, and the size-cap and ratio vectors' bin payload is borrowed, so the rejected read requests nothing from the allocator and the check could never pass. probe_catches_a_reserve_first_reader remains the positive control that shows the probe counts this thread's allocations. --- src/read_allocation_probe.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/read_allocation_probe.rs b/src/read_allocation_probe.rs index 6fc88f3..84a17a7 100644 --- a/src/read_allocation_probe.rs +++ b/src/read_allocation_probe.rs @@ -136,10 +136,10 @@ mod tests { let (envelope, original_size) = reject_vector(name); let (result, requested) = retrieve_counted(&envelope); assert_eq!(result.err(), Some(expected), "[{name}] wrong rejection"); - // The read allocates (the pre-scan's nesting stack; these - // vectors' bin payload is borrowed), so a probe that sees - // nothing is not counting this thread's allocations. - assert!(requested > 0, "[{name}] probe saw no allocation at all"); + // The read can request nothing at all (the pre-scan keeps these + // vectors' nesting on the call stack and their bin payload is + // borrowed), so `probe_catches_a_reserve_first_reader` is what + // shows this probe counts this thread's allocations. assert!( requested < original_size, "[{name}] the read requested {requested} B from the allocator, \ From 3e5b5ea4a9c8bdcd5a679805d93ccd959ce1ecc6 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 18:04:45 +0000 Subject: [PATCH 6/7] docs(readme): restate the retrieve LTO cost against the cheaper pre-scan --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c90a457..8f6e1a6 100644 --- a/README.md +++ b/README.md @@ -370,7 +370,7 @@ The Criterion suite in `benches/hot_path.rs` needs the `encryption` feature (`ma | `tenant_keyring_decrypt/` | `TenantKeyring::decrypt_indexed`, key derived once at `for_tenant` | | `hkdf` | `derive_domain_key` (takes no payload, so it has no size) | -Sizes are 64 B, 1 KiB and 64 KiB. `store`, `retrieve` and `retrieve_data` also run at 40 B, below the 64-byte literals-only block threshold, and at 1 MiB. `ByteStorage` switches to its copying allocation pattern once the envelope or the compressed payload reaches 64 KiB. The 64 KiB payload compresses to a 24,702-byte envelope and never gets there; the 1 MiB one compresses to 383,893 bytes. One case at a time: `make perf-ir ARGS="--case store/1024"`. The bench binary is built with fat LTO, so its cases shape each other's code. Because both `retrieve` and `retrieve_data` call the envelope decode, it stays out of line, and `retrieve` costs 68 to 77 Ir/op more at 40 B to 1 KiB (2.6% at 40 B) than in a binary that calls `retrieve` alone. A consumer that calls only one of the two gets the decode inlined. +Sizes are 64 B, 1 KiB and 64 KiB. `store`, `retrieve` and `retrieve_data` also run at 40 B, below the 64-byte literals-only block threshold, and at 1 MiB. `ByteStorage` switches to its copying allocation pattern once the envelope or the compressed payload reaches 64 KiB. The 64 KiB payload compresses to a 24,702-byte envelope and never gets there; the 1 MiB one compresses to 383,893 bytes. One case at a time: `make perf-ir ARGS="--case store/1024"`. The bench binary is built with fat LTO, so its cases shape each other's code. Because both `retrieve` and `retrieve_data` call the envelope decode, it stays out of line, and `retrieve` costs 68 to 77 Ir/op more at 40 B to 1 KiB (2.7% at 40 B) than in a binary that calls `retrieve` alone. A consumer that calls only one of the two gets the decode inlined. **Method** (`benches/perf_ir.rs`): each case runs in its own process under `valgrind --tool=cachegrind --cache-sim=no`, once with N calls and once with none, after the same setup and one warm-up call. N is 1,000, or 20 for a 1 MiB case. Ir/op is `(Ir[N] - Ir[0]) / N`, so process start, setup and exit cancel. The counts are exact, so N sets the run time, not the precision. The measured process gets an empty environment, so neither the shell nor a `VALGRIND_OPTS` changes what is counted. From 4544d24f8b420e986250a3556ec6869845b0fdc2 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 31 Oct 2026 18:28:24 +0000 Subject: [PATCH 7/7] =?UTF-8?q?fix:=20address=20coderabbit=20review=20?= =?UTF-8?q?=E2=80=94=20scope=20the=20no-allocation=20claim=20to=20the=20wa?= =?UTF-8?q?lk?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit-Resolved: msgpack_bounds.rs:30:Qualify the no-allocation --- README.md | 2 +- src/msgpack_bounds.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8f6e1a6..cda67a8 100644 --- a/README.md +++ b/README.md @@ -266,7 +266,7 @@ allocating in proportion to any declared length. A document nesting at most 8 levels deep is walked with no allocation at all. That covers every envelope `store()` writes, which nests 2 deep; a hand-built envelope can nest deeper in a field the decoder ignores, and its walk then allocates for the levels past 8. -A rejection is +A rejection, which allocates its error message, is `ByteStorageError::DeserializationFailed` with the message prefix `decode pre-scan: `. The same walk is public as `check_msgpack_structure(bytes, max_depth)` for callers that decode untrusted diff --git a/src/msgpack_bounds.rs b/src/msgpack_bounds.rs index 5601d22..42c9c5d 100644 --- a/src/msgpack_bounds.rs +++ b/src/msgpack_bounds.rs @@ -28,7 +28,8 @@ fn reject(reason: impl Into) -> MsgpackStructureError { /// at most `min(max_depth, bytes.len())` of them (8 KiB at a depth of 1024), and /// allocates nothing proportional to any declared length. The first 8 live on /// the call stack, so a document that nests no deeper than that (an envelope -/// `ByteStorage::store` writes nests 2 deep) allocates nothing at all. +/// `ByteStorage::store` writes nests 2 deep) is walked with no allocation. A +/// rejection allocates its error message. /// /// Bytes after the document are neither read nor rejected. To find where the /// document ends, call [`msgpack_document_len`], which runs this same walk.