Chunk abstraction for collections, merge batching, and batches.#744
Merged
frankmcsherry merged 9 commits intoJun 23, 2026
Merged
Conversation
frankmcsherry
force-pushed
the
chunk_basis
branch
6 times, most recently
from
e7ee7c0 to
33b8d3f
Compare
frankmcsherry
force-pushed
the
chunk_basis
branch
from
33b8d3f to
4475f8a
Compare
col_chunk was an early columnar Chunk mirroring ord_neu; the phase-2 TrieChunk supersedes it and fixes its recorded limitations (decompress/recompress merge, the singleton-vs-logical-count prefix re-walk, val-boundary-only cuts). Carrying it taxed every chunk_basis modification with a re-application to a full Chunk impl slated for deletion, so it goes early. The chunks example drops its `colchunk` mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y updates Two merge-batcher / chunker fixes for the bfs/probe regression (plan 1.5, 1.4): 1.5 — VecChunk's SizableContainer absorbs to TARGET updates (len >= TARGET, ensure_capacity reserves TARGET) instead of timely's byte-derived buffer size, so chunks arrive pre-graded rather than re-melded downstream. 1.4 — MergeBatcher's geometric ladder weighs chains by summed updates, not chunk counts: regrading decouples the two, so a trickle of single-update chunks re-merged the head chain on every insert. A chain is immutable until merged, so the weight is cached alongside it (chains: Vec<(usize, Vec<Chunk>)>). Also refocuses the Merger trait: the bundled account() -> (records, size, capacity, allocations) splits into len() -> usize (update count, drives the ladder and the logger's records field) and a defaulted allocation() -> (size, capacity, allocations) for memory telemetry. BatcherEvent's shape is unchanged. NOTE: breaking change for out-of-tree Merger implementors (e.g. Materialize) — rename account -> len, optionally override allocation. chunks 100k/200k (u64 probes): ~127ms -> ~100ms queries-complete. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1.2 — invert the merge surface so binary merge_pair is the required primitive: merge_pair(&mut (usize,Self), &mut (usize,Self), out) is what the harnesses (merge_chains, the batch merger) drive directly; the k-way `merge` becomes a provided dispatcher (arity 0 / 2 / unimplemented!); `prune` loses its default (it was the only arity-1 merge caller). Backends with a genuine k-way merge (vec_chunk's merge_buf) override `merge`. 1.3 — vec_chunk::merge_pair is a dedicated two-pointer binary merge: one gallop pins the horizon, disjoint runs bulk-copy via extend_from_slice, only collisions consolidate element-wise. merge_buf stays as the k-way override and correctness reference. Chunks example ~100ms -> ~83ms. Fixes a latent bug in the spike's run-copy: it pinned the horizon at the lesser last (key,val), which is wrong for multi-chunk chains — a (key,val) group that straddles a chunk boundary on one side and overlaps times with the other gets the other side's whole group merged before its straddled continuation arrives, emitting duplicate, out-of-order (key,val,time) entries. The horizon must be (key,val,time), as the reference merge_buf already used. The spike's merge tests are single-chunk and never reached it. New property test merge_pair_matches_reference: 300 random multi-chunk merges (tiny chunks force straddling) vs a union-consolidate oracle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
frankmcsherry
force-pushed
the
chunk_basis
branch
from
683d927 to
9cd111c
Compare
merge / extract / advance / regrade had four different calling conventions and three support types (ChunkList, ChunkFeed, plus bare Vec); the variation was mostly incidental. Collapse them onto one stream-transducer protocol: consume from the front of a VecDeque<Self> input; withhold anything not yet committable by reforming it into one owned chunk and push_front-ing it back; append committed chunks to a VecDeque<Self> output; `done` forces the flush. Cross-call state is now only the deques — no index escapes a call. Concretely: * merge is binary, `(in1, in2, out)`, no `done`: it merges the two front chunks through their shared horizon, pops the drained side, and prunes+push_front's the partial one. The harness (merge_chains / ChunkBatchMerger) handles a drained input by flushing the other side's verbatim tail, so merge never reasons about end-of-input. The persisted `(usize, Self)` slot and the k-way `merge`/`merge_buf` are gone; `prune` is now an inline drain, off the trait. * advance / regrade keep `done` and withhold via push_front (advance's last group; regrade's sub-TARGET carry). advance reuses the front chunk's storage in place, so a giant key stays linear. * extract stays the one-shot splitter: drains its whole input, two outputs plus the residual frontier, no `done`. Grading is now a seal-time property, not a between-stage invariant: the producers emit near-graded output and a terminal regrade coalesces the seams. ChunkList (sink + regrade-on-push) splits into a raw VecDeque plus an explicit regrade stage; ChunkFeed (whose usize was dead for advance) and AdvanceQueue are deleted. ChunkBatchMerger becomes a fixed merge -> advance deque pipeline (regrade at done()); ChunkBuilder drives regrade directly. Net -176 lines. All chunk tests pass; the chunks example is unchanged at ~75ms vs ord's ~98ms (100k/200k). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The one-front-pair merge pruned and pushed back the partial side on every call, so a chunk straddling many chunks on the other side was re-pruned (copied) once per straddled chunk — a measurable ~4% queries / ~10% loading regression at 1M versus the pre-deque baseline. Have merge instead drain both deques' loaded content in one call: walk across chunk boundaries with local indices (p1/p2) reset as each working chunk is retired, reading through the shared Rc (never `take`n, so a source clone is not deep-copied), and prune the partial side's suffix exactly once at the yield boundary. The merge harness (ChunkBatchMerger) now loads a burst of chunks per side (BURST=8) so that single prune amortizes over the burst; merge_chains (the batcher) already feeds whole chains, so it drains in one pass for free. The drain is a `while p1 < c1.len() && p2 < c2.len()` guarding the cursor indexing, refilling exhausted working chunks at the foot of the loop — no bare `loop`, and the cursor state stays in locals (an earlier `refill(&mut …)` helper spilled it to memory and cost ~3%). Recovers to pre-deque parity: 1M/2M ~205ms loading / ~643ms queries, ~1.6x faster than ord_neu. All chunk tests (incl. the 300-case merge property test) and the full suite pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ar risk Addressing review feedback on the phase-1 foundation; all doc/comment, no logic. * merge: delete the duplicated "working chunks" paragraph (copy-paste residue from the loop→while cleanup). * BURST: a sweep shows it is insensitive on this workload (1..32 flat to ~noise at 1M), so the recovery was the drain-merge reading through the Rc + copying only the partial suffix, not the burst. Rewrite the comment to say what BURST actually trades (fuel granularity vs. re-pruning a pushed-back partial under straddle-heavy inputs) and that 8 is a conservative default, not a tuned value. * ChunkBatchMerger: state the latency bound explicitly — fuel bounds each step to ~one burst-merge, but the terminal advance(done) + regrade ride outside fuel; worst case (one (key,val) spanning the whole merge) is one unfueled step, kept linear in the group by vec_chunk's in-place carry reuse. A chunk impl must keep that flush linear. * vec_chunk docs: spell out that the protocol is layout-agnostic but the merge *body* is not — it bulk-copies a contiguous slice; a columnar chunk must range-copy the key/val/time/diff columns with offset bookkeeping (the operation that beats flat on repetitive keys, and where the removed col_chunk struggled). Nothing here exercises a columnar merge — that body is the open phase-2 risk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the worked `vec_chunk` reference impl (and its tests) out of the 1.5k-line `chunk.rs` into `chunk/vec_chunk.rs`, leaving `chunk/mod.rs` holding just the `Chunk` trait and the generic harness. This makes room for sibling impls (e.g. a columnar `col_chunk`) without a single massive file; the module's own docs already frame `vec_chunk` as the reference shape a next implementor follows. Also drop the `= 1024` default on `Chunk::TARGET`, making it a required associated const. The right value is layout-dependent (it trades against the row-major vs. columnar crossover), and the sole impl already set it explicitly, so the default had no users and only risked a silent, mistuned inheritance for the next implementor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename `regrade` to `settle`: the point a chunk leaves the active set, where it grades and may later compress or spill. Settle as output is produced — in merge, extract, the batch merger's `work`, and the builder — so the un-settled in-core set stays bounded rather than settling everything at seal. Redefine `extract` to do bounded work per call, with the harness settling each side between calls. Take `AntichainRef` in `extract`/`advance` to match the `Merger` contract and drop a per-seal allocation. Rename the public builder alias to `ChunkBuilder` (inner impl `ChunkBatchBuilder`), keeping the `Rc` wrapper out of the name. Add module docs (wiring, bounded footprint) and a resumable-merger reference test; tighten comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
This PR introduces a
trait Chunkthat can be used to back collections, merger batchers, and in a list back a whole batch itself. This is meant to streamline implementation, and remove unnecessary distinctions. Also a bit of an experiment, to see just how small these implementations can get. At the moment .. not tiny .. but not enormous either.