A reference VecReduceBackend, tests, and a tactic-vs-tactic benchmark - #825
Merged
Conversation
…mark A ProxyReduceBackend over hash-keyed VecChunk storage, the plain-rows counterpart to the corgi backend. It covers the key space in bounded windows, so the harness's multi-window path — the code #824 noted had never executed — now runs under its contract assertions in every test. Input value ids are per-run ordinals from a shared pool: the history and novel runs mint independently, and a value present in both gets two ids, reconciled because reduce_corrections resolves ids to values and consolidates by value before applying logic. Output ids are interned, sharing the namespace corrections mint into. Collisions re-group each hash bracket by real key, so a 64-bit collision is an inefficiency rather than an error (tested by forcing every key to one hash). tests/int_proxy.rs gives the proxy tactic its first in-repo tests: a direct retire, collision correctness, cursor-reduce comparisons (flat, String values, multi-moment, one-key windows), and reduce inside iterate. tests/int_proxy_bench.rs benchmarks the cursor tactic against the proxy tactic over the SAME hash-keyed arrangement (with the inherent reduce as reference) across three regimes: streaming churn, multi-time batches, and label propagation inside iterate with churn — the carried-interesting-times shape of #824. Current readings, single worker, release: churn 1.03x cursor-same; multimoment 1.84x; propagate 3.39x (output checksums asserted equal across all three modes). Also adds VecChunk::as_slice, a read-only accessor for the sorted records, which the backend's merge walks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fast path in `reduce_corrections` skipped the per-key regrouping when a bracket's first and last input ids resolved to the same real key, and likewise for its output ids, on the stated grounds that "ids resolve in key-sorted order". Neither id space is key-ordered across a bracket. Input ids are ordinals minted from one pool, the history run first and then the novel run, so the order is `history by key, then novel by key` rather than globally by key. Output ids are interned, with the corrections a crossing mints appended after the presentation's. Either can read `[A, B, A]` by id, whose endpoints agree while its interior does not, and the bracket is then reduced as if it were all `A` — so a hash collision became an error rather than the inefficiency the general path exists to make it. Resolving every id in the bracket and requiring them all to agree is a linear pass over data the fast path clones immediately afterwards, against the general path's per-key maps. The three benchmark workloads are unchanged by it (churn 1.03x, multimoment 1.79x, propagate 3.93x against the cursor tactic, all within run-to-run noise of the previous numbers). `reduce_collision_fastpath_endpoints` is the regression test: a reduction that emits only for `Collide(1)` leaves `Collide(2)` with input but never any output, so retire 2 presents the input bracket as `[C1, C2, C1]` and the output bracket as `[C1]`, and both endpoint tests passed. Before this commit it produced `(Collide(1), 900)` — the other key's value — where `(Collide(1), 6)` was correct. `reduce_collision_across_retires` covers the neighbouring shape that does reach the general path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every key's input nets to zero by time 2, so from then on it holds no records in any of the three presentations while still being a key the retire must consider: its stale output has to be retracted. With one key per window this is also the case where a window's key list and the `changed` set disagree, which the harness's window-key derivation depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`merged_run` seeked each batch once, to the window's first key, and then walked forward record by record — stepping over every record of every hash it did not want. So a call cost the accumulated history, not what it was asked for. On `propagate` that is 64,436,421 records scanned to present 441,218, a factor of 146, across 11,739 calls asking 63,318 keys in total: about five scattered keys per call, each call re-reading the whole trace. `churn` and `multimoment` ask for nearly every key at once, so scanning everything IS presenting everything and they waste nothing — which is why the shortfall looked like a property of the tactic and tracked moments-per-key. Skipping now seeks: when the least hash present is not wanted, every batch binary-searches to the next key that is. That lands at or above it, so at most one unwanted hash is visited per wanted key. No threshold to tune, because a dense key set never takes the branch — unlike corgi's `collect_present`, which chooses between seeking and scanning on a measured ratio. propagate 10361us -> 3124us/round 3.76x -> 1.12x of the cursor tactic churn 0.95x -> 0.94x multimoment 1.66x -> 1.59x This is the yardstick, not the thing being measured: `merged_run` is the reference backend's, and every proxy-vs-cursor number taken before this was reading its cost as the tactic's. It belongs on `vec-backend-recipe`, where `vec_backend.rs` lives; it is here because that branch is cherry-picked in and this is where it was measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`reduce_corrections` minted an id for every emitted correction by looking `(key, value)` up in a `BTreeMap`. Carving the call into phases put 435 of its 513 samples there — more, on its own, than the whole of the conventional tactic's `compute`. The clones are not the problem; for these workloads the key is two `u64`s and copying it is free. The B-tree descent is. churn 0.94x -> 0.58x of the cursor tactic (47.5ms -> 29.1ms/round) multimoment 1.59x -> 1.09x (32.1ms -> 21.0ms/round) propagate 1.12x -> 1.13x (unchanged) Propagate is untouched because its corrections are few; churn and multimoment emit one per key per moment, which is where a per-correction map lookup lands. The backend now asks `Hash` of its key and output types, which is what a backend that interns needs and no burden on one that does not — the corgi backend derives ids by content-hashing a column and keeps no map at all. `Collide` states its collision as a `Hash` that writes a constant, rather than as a `Hashable` impl, since `Hashable` has a blanket impl for `T: Hash` and the two cannot coexist. Third finding in a row in the reference backend rather than the tactic. The benchmark was built to compare tactics over identical storage and has been comparing a young backend against a mature cursor implementation; ratios taken before these three fixes read its cost as the tactic's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three small things a reviewer raised. The continuation after `partition_point` in the seek branch is unreachable: chunks are non-empty and the guard above skipped any whose last key is below the sought one, so the chunk holds a record at or above it. A `debug_assert` states that, and the branch stays, so a violated invariant degrades to a slower walk rather than to a batch that silently reads as drained. `out_ids` says it is lookup-only and that its iteration order is never observed, which is what makes a `HashMap` safe here. The key set was rebuilt when `from` opened at `Some(0)`, which is true today but is not something the trait promises. It now rebuilds on a flag that `begin` sets — `begin` runs once per retire, before any window — so the backend no longer depends on the value the harness opens with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI denies warnings; my local runs did not, so this only surfaced there. Checked the whole workspace under `-D warnings` rather than just the reported line — this was the only one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`finish` consolidated each tile's whole accumulation, sorting an array that was already in key-hash order. It only ever needed ordering within a hash: interned ids are in first-seen order rather than value order, so a hash's records can need reordering by their real `(key, out)` value, but no record ever needs to move past a different hash. `emit` is the place for it. A call carries the whole of every hash it mentions — the harness consolidates a tile's deltas per window, and a hash belongs to exactly one window — and calls arrive in ascending hash order, so consolidating the run just appended leaves the tile ordered. A `debug_assert` states the ascending-arrival property the backend now relies on. The difference is a sort per emit against one sort of everything a retire produced, which is invisible at benchmark scale and is not at the scale this is meant for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The workload sizes and round counts come from the environment, and `PROG`/`MODE`
run a single cell of the matrix. Costs that are structural but invisible at a few
megabytes — a sort over a whole retire's output rather than over each key's run,
say — need a run that moves gigabytes to register, and the suite could not be
asked for one without editing it.
CHURN_KEYS=4000000 ROUNDS=6 WARMUP=2 PROG=churn MODE=proxy \
cargo test --release --test int_proxy_bench -- --ignored --nocapture
Defaults unchanged, so the reported matrix is the same.
Co-Authored-By: Claude Opus 5 (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
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.
A reference
ProxyReduceBackendoverVecChunkstorage, the first tests DD has for the proxy reduce tactic, and a benchmark that compares the two reduce tactics on identical storage.Work with the backend has surfaced a few interesting performance gotchas, which were then fixed up, but .. probably need ongoing attention if this is meant to be used as a benchmark (Claude imagines that's a good idea; I'm not as convinced).