perf(#30): zero-copy stream parsing + test/coverage/mutation safeguards#63
Merged
Conversation
Research synthesis (5-track parallel sweep) + approved design for issue #30. Decision: keep hand-rolled parser; eliminate the 29 Bytes::copy_from_slice sites in src/stream by threading a shared Bytes through the group parsers and slicing instead of copying. Copy-once at the outermost boundary; codec path stays zero-copy. Borrowed Matter<'a>/winnow adoption explicitly deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
10-task TDD plan: fix quadlet arithmetic, add Bytes core, convert leaf/nested/ quadlet parsers to slice, copy-once wrappers, zero-copy codec, unwrap, Groups copy-once, benchmark, gate. Aliasing pointer-range tests as the bug-probe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…roup with checked_mul Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename parse_quadlets test to reflect it verifies huge-count/insufficient-input returns NeedBytes without panicking (defensive boundary), not the checked_mul overflow arm (unreachable on 64-bit usize). Add equivalent test for parse_quadlets_v2 and a comment noting the overflow arm guards wasm32. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rsers) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…copying Includes aliasing bug-probe test proving the group raw is a slice of the parent buffer, not a fresh allocation. Old dispatch keeps a temporary copy-adapter until the dispatch is fully converted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ying
Convert every remaining CESR group parser to the zero-copy `&Bytes`
convention: each `parse`/`parse_v2` now takes `input: &Bytes` and returns
`(T, Bytes)` via O(1) `input.slice(..)`, dropping the per-call
`Bytes::copy_from_slice`. Rebuild the group dispatch on top of the
`&Bytes` core:
- Leaf/nested parsers (witness_idx_sigs, non_trans_receipt_couples,
first_seen_replay_couples, seal_source_{couples,triples,last_singles},
digest_seal_singles, merkle_root_seal_singles,
backer_registrar_seal_couples, typed_digest_seal_couples,
blinded_state_quadruples, bound_state_sextuples, typed_media_quadruples,
trans_receipt_quadruples, trans_idx_sig_groups, trans_last_idx_sig_groups,
attachment_group) now &Bytes-based; skip-loops unchanged.
- quadlet_group: GroupParser is `fn(&Bytes) -> (CesrGroup, Bytes)`;
parse_quadlets/_v2 slice instead of copy and pass parse_group_bytes/_v2;
the iterator slices the payload per step.
- dispatch_v1/v2/v2_quadlets/v2_special are &Bytes-based and pass parser
results straight through; the temp copy-adapters and the
dispatch_v1_bytes/dispatch_v2_bytes bridges are deleted;
parse_group_bytes/_v2 lose their dead_code allows.
- parse_group_inner/_v2 become copy-once shims over the Bytes core; codec,
encode, and unwrap call sites use parse_group_bytes/_v2.
The only copy_from_slice left on the parse path are the two copy-once
shims (and unchanged test/types code). All 1696 tests pass; clippy clean
with zero new allows.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Non-quadlet decode now snapshots the buffer as Bytes and parses zero-copy via parse_group_bytes, reattaching only the unconsumed tail; the common single-frame case performs no copy. Also replaces saturating_mul/unwrap_or with checked_mul. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
restore_buf drops buf's residual shared handle (via mem::take) before try_into_mut, so the snapshot allocation is reclaimed in place instead of always falling into the copy fallback. DRYs the four restore sites into one helper. Adds a same-allocation test (buf.as_ptr() unchanged after NeedBytes) and a byte-exact content assertion to decode_incremental so a corrupted retained byte is caught, not just a length change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es slicing Loop now parses via the Bytes core (no whole-buffer copy per iteration); entry and nested-group extraction slice the parent Bytes instead of copying. Adds QuadletGroup::raw() O(1) accessor. Eliminates the consumed-length subtraction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tency Crate-wide raw() returns borrowed &[u8] (Matter/Siger/Indexer); the new owned Bytes accessor is named to_bytes() to avoid contradicting that contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Eliminates the per-next() whole-buffer copy (O(N^2) over N groups); iterators now copy once lazily and hand out O(1) Bytes slices. Public signatures unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…overage Adds aliasing (slice-not-copy) tests for couple/quadruple/quadlet parser shapes and the previously-untested GroupsV2 iterator (multi-group, stops-on-error, empty, copy-once). Each mutation-proven. No production changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Formatting drift from the conversion tasks (per-task checks ran clippy+nextest but not rustfmt). No semantic changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ant to group count Counting global allocator in a dedicated test binary asserts iterating N groups copies the region ONCE (allocation count independent of N); fails if any parser regresses to per-group copying. Catches silent correct-but-copying regressions that conformance tests cannot see. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rage Adds coverage for previously-untested V2 group dispatch (dispatch_v2 leaf arms, dispatch_v2_quadlets quadlet arms, dispatch_v2_special genus rejection) and the codec quadlet_to_group_v1/v2 mappings plus decode_v2 frame arithmetic. Also pins parse_group_v2's returned remainder via a trailing-bytes assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
QuadletGroup: exact NeedBytes shortfall, exact-size success boundary, and two-inner-group iteration (kills the cursor-advance and next() guard mutants). unwrap: trailing-sibling preservation (the dropped `!`) and the exact-8-byte genus-counter offset boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds packages.coverage (craneLib.cargoLlvmCov, --all-features HTML) and a post-merge coverage workflow. On-demand via 'nix build .#coverage', NOT a gating check (instrumentation recompiles the world). llvm-tools-preview is already in the toolchain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l counters)
The counting global allocator used GLOBAL atomics, so under a thread-parallel
runner (plain `cargo test`, which `cargo llvm-cov` uses) the two invariance
tests running concurrently in one process polluted each other's `measure()`
delta with the other thread's allocations — non-deterministic failures like
"14 allocs for 2 groups vs 4 for 8". nextest's process-per-test isolation hid
this; coverage (`nix build .#coverage`) tripped it.
Switch the counters to const-initialized THREAD-LOCAL cells so each `measure()`
sees only the calling thread's allocations. `const { Cell::new(0) }` init is
non-lazy (never allocates on access) and `try_with` guards TLS teardown, so the
counters are safe to touch from inside the global allocator with no re-entrant
allocation. Robust under serial, thread-parallel, and process-per-test runners
alike; numbers unchanged (2 allocs for both 2 and 8 groups).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parses N back-to-back groups (N=1..256) via groups(); per-group cost stays flat (O(N)) with copy-once slicing. A regression to per-group copying would show as super-linear growth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The buffer was Arc-cloned every next() and then sliced (two refcount bumps); only the per-group slice is needed. Behavior unchanged (copy-once aliasing + allocation-invariance tests still pass). Small win on the sync groups() path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Documents the zero-copy refactor (non-breaking public API), the measured small-stream perf tradeoff, and the test/coverage/mutation safeguards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merging this PR will degrade performance by 24.74%
Performance Changes
Comparing |
… win Removes the false O(N) vs O(N²) claim (origin/main was already O(N)) and the throughput framing. States the measured cost honestly: ~22-28% parse regression on small streams (CodSpeed), accepted in exchange for ~N->1 allocations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
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.
P1.3 · Zero-copy stream parsing (#30)
Makes the
stream/group parser slice a sharedbytes::Bytesinstead ofBytes::copy_from_slice, cutting heap allocations per multi-group message from ~N to 1 (0 on the codec path). Public API is unchanged (non-breaking).Research → design → plan are on the issue and in
docs/superpowers/. Decision (5-track research sweep): keep the hand-rolled parser (nowinnow/nom), useBytes+.slice()(no lifetime leaks into the public API), defer borrowedMatter<'a>.What changed
&Bytesand return(T, Bytes)via.slice()—skip_*loops byte-for-byte unchanged; only copy→slice + signature differ. Dispatch rebuilt onBytes(all 44 code→variant mappings verified identical).decode_v1/v2): non-quadlet path parses zero-copy on success; theNeedBytesrestore reclaims the buffer in place (fixed a deadtry_into_mutthat always copied). Bannedsaturating_mul/unwrap_or→checked_mul.unwrap_generic_groupandGroups/GroupsV2slice a once-copied region instead of re-copying.QuadletGroup::to_bytes()— O(1) shared-buffer accessor (additive).Performance — measured, honestly
tests/allocation.rs; a per-group-copy regression makes it scale (8 groups: 2 → 18 allocs; 760 → 4264 bytes).controller_idx_sigs_1sig−27.7 %,multi_group_controller_witness−21.7 % (aggregate ≈ −24.7 %). Confirmed by a localorigin/main-vs-branch run (single-group +58 % at N=1). The Arc/indirection overhead is per-parse; it amortizes — per-group cost is ~equal tomainby N≥16 groups (both are O(N);mainwas never O(N²)).Test / coverage / mutation safeguards (the bulk of this PR)
Behavioral tests can't see "correct-but-copying" regressions, so this adds property-level guards:
--test-threads=1/ nextest).cargo-mutants(dev shell, on-demand): swept the core stream logic — 161 mutants, 100 % of non-equivalent mutants killed (2 full sweeps). Found and closed a real gap: the entire V2 dispatch table + codec quadlet mapping was untested (~35 arms deletable with everything green).nix build .#coverage(llvm-cov) + post-merge workflow (mirrorsbombay). Not gating.Bug fixes included (valuable independent of the perf tradeoff)
saturating_mul/unwrap_orarithmetic →checked_mul(quadlet_group, codec).NeedBytes/Errrestore now actually reclaims in place (was silently copying).Constraints held
no_std + alloc and wasm32 green; no new deps; no
unwrap/expect/panic/asin production; strict clippy clean;nix flake checkgreen.Note: the CodSpeed check is red by design — it's the accepted allocation-for-CPU tradeoff, not a mistake. Acknowledge/annotate on CodSpeed before merge.
Closes #30.
🤖 Generated with Claude Code