perf(fast): incremental doc-top matches in the connected delta path#67
Merged
Conversation
The 10x windowed profile showed ~1M docs across a 500-merge Boundless run taking the Rebuilt fallback (full seq_recount + full-map diff of ~500-child docs) because any doc-top cross-word match bailed out of try_apply_incremental. Top matches are now handled incrementally: greedy left-to-right non-overlapping collection (exactly seq_try_merge's order), consumed children rebuilt into a new children Vec with delta positions recorded, boundary accounting extended with the affected pair-starts around each match. The no-match in-place fast path is preserved; rare full collapse still falls back. Rebuilt count in the early windows: ~190k-400k docs per 100 steps -> 0. 10x (wikitext-103 slice, 500 merges, cold), digests unchanged: - SuperBPE: 24.1s -> 18.3s (-24%), RSS -0.9GB (its connected phase 2 is cross-word-match-dense - the biggest beneficiary) - Boundless: 31.6s -> 28.2s - BPE/BNE: unchanged within noise Tried and reverted: fold-emitted per-shard op buckets (flat, +1GB transient RSS), CAND_SHARDS 32 (noise). Co-Authored-By: Claude Fable 5 <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.
Boundless round at the 10× workload (its measured home: 31.2s, pure loop). The windowed profile confirmed the top-ranked hypothesis decisively: ~1M documents across a 500-merge run took the Rebuilt fallback — full
seq_recount+ full-map diff of ~500-child docs — because any doc-top cross-word match bailed out of the incremental path.Change
try_apply_incrementalnow handles doc-top matches incrementally: matches collected greedy left-to-right non-overlapping (exactlyseq_try_merge's order), consumed children rebuilt into a new children Vec with positions recorded, and the candidate delta extends the existing boundary accounting ({i−1, i, i+1}pair-starts around each match on the old side; new-side pairs read off the rebuilt Vec). The no-match in-place fast path is untouched; rare full-collapse still falls back. Rebuilt docs in the early windows: 190k–400k per 100 steps → 0.Results (10×: 237,721 docs / 108.9M chars, 500 merges, cold; digests unchanged everywhere)
SuperBPE is the surprise beneficiary — its connected phase 2 is cross-word-match-dense, so killing the fallback pays more there than in Boundless itself.
Tried and reverted: fold-emitted per-shard op buckets from the diff workers (flat, +1GB transient RSS), CAND_SHARDS 16→32 (noise).
Verification
10× digests identical (
510a924cfc/407ab745c8/4f8d7bc7cd/9b16e730a0); 2000-row and full-scale/500 canon unchanged; both suites 147 passed, 2 skipped; clippy unchanged. Diff:trainer.rs+104/−18.Honest re-ranking of what remains
The profile now shows true op volume as Boundless's wall (~1M count updates/step early — 113k affected docs × ~10 changed candidates each), not discovery overhead. That demotes the long-standing postings-index idea (discovery is already delta-driven). What's left, in order: (a) aggregating same-candidate global increments before map ops, (b) the deep fix — a flat u32-word-id doc representation to cut memory bandwidth on the 7–8GB working set (a much bigger refactor). Also noted: ±0.5s / ±1.2GB run-to-run variance at 10× from allocator behavior — worth knowing before chasing sub-second wins.
🤖 Generated with Claude Code