Drive VecReduceBackend windows from a resumable merge-walk - #829
Merged
Conversation
MergedWalk replaces merged_run: a resumable cursor over one logical batch list, whose first `novel` entries are the retire's novel input and whose rest are its prior history, holding a (chunk, offset) per batch, an index into `changed`, and the next active hash. Only the novel prefix and `changed` activate a key; prior batches are seeked to whatever key the active set names, so a window stays proportional to the work asked for rather than to the accumulated trace. Windows are now formed lazily. The active keys are the merge of the novel batch heads with `changed`, so the whole-trace prescan that built keys_cache is gone, along with keys_stale; keys_cache now holds only the current window's keys, which the output pass replays as its own active set. The budget is spent in records rather than keys, since records are what the presentations cost, and it is checked at key boundaries so a key is never split across windows. Prior records are advanced to the compaction frontier as they are drawn and the draw is consolidated per (value, role, time). Advancement without consolidation cannot reduce anything, which is why the frontier bought nothing before; together they remove 46% of the presented records on the benchmark. Identifiers are minted from one namespace shared by both input runs and only after consolidation, so a novel retraction cancels against its own history and a value whose records all cancel spends nothing. Times and diffs move into the bridges rather than being cloned into them, leaving one owned time per presented record. Seeks are searches, the chunk advance included, and a run's end is found by doubling rather than by bisecting the records behind it. Both carry a fast path for an already-positioned cursor: searching the dense path rather than comparing cost 1.71x on churn. Against the previous backend: multimoment 21.2ms -> 17.5ms, wide 31.3ms -> 26.4ms, churn 27.6ms -> 25.7ms, propagate 3.03ms -> 3.15ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The proxy backend advances the accumulated input and output to `lower` as it loads them, which is sound only because no time below `lower` is ever evaluated or used as a comparison target. That was true but stated nowhere, so the backend depended on an invariant a reader could not check. `ReduceTactic::retire` now says what the interval bounds: every time a tactic evaluates at lies in `[lower, upper)`, times at or beyond `upper` are deferred rather than evaluated, and times below `lower` were evaluated by an earlier call. The bound is one-sided in the sense that matters — a sweep's frontier does reach past `upper`, to defer what it finds there — so the clause speaks about evaluation rather than about every time visited. `ReduceInstance::lower` now carries the consequence for a backend: it may and should advance by the frontier, but only together with a consolidation, since advancement alone rewrites times and removes no records. The novel input is exempt, as its times are the interesting-time seeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whether a hash carries more than one real key was rediscovered inside `reduce_corrections`, which resolved every identifier of every bracket on every wave to find out. The walk already sees each value once, so the question is cheaper to answer there: the drain loop's "the value changed" branch fires once per distinct value, and testing the real keys for agreement costs one comparison there. A window now records `collisions`, the hashes carrying more than one real key, and `reps`, one representative input identifier per key. `reduce_corrections` consults the set and reads the key off the bracket's first identifier, which is sound given the set and would not be without it: neither identifier space is key-ordered across a bracket, so both can read `[A, B, A]`, whose ends agree while its interior does not. Corrections cannot invalidate the set, as they mint rows under a key the presentation already showed. `reps` exists because a collision can hide from either pass alone. A hash whose input has fully cancelled for one real key still carries that key's stale output, so an input pass seeing only `A` and an output pass seeing only `B` are each internally consistent. Reaching that state needs a correction deferred past a retire's upper bound, which no test here constructs; the check is insurance against a wrong answer rather than tested behaviour. Two tests cover the rest. `reduce_collision_multiwindow` buckets several real keys into each of two hashes and spans several windows, so the collision set is built and discarded more than once per retire. `reduce_collision_inside_iterate` does the same under `Product` times. Suppressing the collision flag fails all five collision tests. churn 25.7ms -> 25.5ms, multimoment 17.5ms -> 17.2ms, wide 26.4ms -> 26.1ms, propagate 3.15ms -> 3.10ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rustc partitions codegen units along module boundaries, and fat LTO does not undo the choices each unit made before it ran. Rewriting one module's contents therefore swings a benchmark by however much the new partitioning happens to differ, in whichever direction. Measured: replacing `merged_run` with a merge-walk in `vec_backend.rs` read as multimoment 21.2ms -> 17.2ms, wide 31.3ms -> 26.4ms and churn 27.6ms -> 25.5ms at `codegen-units = 4`. Rebuilt with one unit, every ratio fell inside 0.98-1.02 with fully overlapping run ranges: the old code alone got ~19% faster, the new code barely moved. The apparent win was the old arrangement partitioning badly. The other tactics in the same binary are not a control for this. The cursor and inherent reductions, untouched by that change, moved at most 4.5% across the two builds while the proxy tactic moved 19% — the damage is confined to the module that was rewritten, which is exactly how the partitioning works. Full workspace release builds in 3m07s with one unit against 3m10s with four, because fat LTO already dominates and the build parallelises across crates rather than within them. A single release test target built alone costs about 24s more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was the only program in the file with its shape hardcoded, so it could not be scaled to ask whether an effect grows with the work. PROP_NODES, PROP_EDGES, PROP_CHURN, PROP_ROUNDS and PROP_WARMUP now read through `sized`, as the other programs already do. The churn retracts edge `r * PROP_CHURN + c`, which has to be one of the edges initially inserted, so an assertion states that. 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.
VecReduceBackendpresented its windows by walking the trace three times, once per bridge, from a key list it built by scanning every record of every novel batch first. This replaces that with one resumable merge-walk, and fixes two things the old presentation got wrong.What changes
One identifier namespace for both input runs.
MergedWalkwalks the novel batches and the prior ones as a single list, so a value carries one identifier whichever run it appears in. Before, the two runs minted independently, so a value retracted by the novel batch could not cancel against its own history: the tactic saw a non-empty input, scheduled a crossing, and called user logic with an input that consolidated to nothing. The module doc described this and asked for "a more attentive identifier selection".Prior records are advanced to the compaction frontier, and consolidated.
ReduceInstance::lowerwas never read. Advancing alone buys nothing — it rewrites times and removes no records — so the two go together; separately, neither is worth doing. Consolidation also closes a latent double-count: two batches advanced to the same frontier but not yet merged can hold the same(hash, value, time), which the old presentation emitted twice, breaking the sorted-bridge invariant in debug and double-counting in release.Windows are formed lazily. The active keys are the merge of the novel batch heads with
changed, taken as the walk goes, so the whole-trace prescan is gone. The budget is spent in records rather than keys, since records are what the presentations cost, and it is checked at key boundaries so a key is never split across windows.Collisions are settled when the window is formed.
reduce_correctionsused to resolve every identifier of every bracket on every wave to learn whether a hash carried more than one real key. The walk sees each value once, so the question is answered there for one comparison per value, and the bracket check becomes a lookup.Performance: no measurable change
Under the shipped
codegen-units = 4this read as multimoment 21.2ms → 17.2ms, wide 31.3ms → 26.4ms, churn 27.6ms → 25.5ms. Rebuilt with one codegen unit, every ratio fell inside 0.98–1.02 with fully overlapping run ranges — the old code alone got ~19% faster while the new code barely moved. The apparent win was the old arrangement partitioning badly, not the new one doing less.The in-binary control tactics do not catch this: the cursor and inherent reductions, untouched, moved at most 4.5% across those builds while the proxy moved 19%, because the partitioning damage is confined to the module that was rewritten. Only a one-unit rebuild separates them, so
Cargo.tomlnow pins it; the full workspace release build is unaffected (3m07s against 3m10s), as fat LTO already dominates.The walk does present 46% fewer records, but that buys no measurable time — which is itself worth knowing before optimising this backend for record volume.
Also here
ReduceTactic::retireandReduceInstance::lowernow state the invariant the advance depends on: every time a tactic evaluates at lies in[lower, upper), so accumulated input and output may be advanced tolowerwhen loaded. It was true and stated nowhere, which left the backend resting on something a reader could not check.The propagate benchmark reads its shape from the environment like the other programs, so it can be scaled to ask whether an effect grows with the work. That is how the codegen artifact was caught.
Tests
Two new:
reduce_collision_multiwindowbuckets several real keys into each of two hashes across several windows, exercising the per-window rebuild of the collision set;reduce_collision_inside_iteratedoes the same underProducttimes. Suppressing the collision flag fails all five collision tests.One check is untested and deliberately kept: a hash whose input mentions only key
Aand whose output only keyBcollides while each pass alone is consistent, so the input pass leaves a representative for the output pass. Reaching that state needs a correction deferred past a retire's upper bound, which no test here constructs. It costs one comparison per key and what it guards is two real keys' values merging into one reduction.