Skip to content

perf(fast): parallelize per-step apply, recount, and delta diff#53

Merged
AmitMY merged 1 commit into
mainfrom
perf/profile-2000
Jul 23, 2026
Merged

perf(fast): parallelize per-step apply, recount, and delta diff#53
AmitMY merged 1 commit into
mainfrom
perf/profile-2000

Conversation

@AmitMY

@AmitMY AmitMY commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Profiled the calibrated benchmark workload (2,000 rows / 100 merges). Phase timers showed all four cases route through train_entries_delta, and the connected loop's cost was apply+recount 2.8s (of it seq_recount 1.8s) + delta bookkeeping 0.9s — all serial. SuperBPE additionally spent 2.35s serially replaying its 50 phase-1 merges in make_trainer.

Rust-side only, per the project rule: the reference stays clean, fast/ gets the machinery. (A set of Python micro-optimizations from the same profiling round bought ~9% on connected mode but was dropped as not worth the added complexity in the reference.)

Changes (fast/src/trainer.rs)

  1. The per-step affected-entry apply+recount loop runs via par_iter_mut (threshold 64 entries, so small steps skip rayon overhead).
  2. The old-vs-new candidate diff is computed per entry in parallel; only the few real changes are applied serially to the shared global/index (updates are additive, and pick_best_entries reads index vectors order-independently).
  3. Trainer.apply_merge parallelizes over subgraphs with memoized try_merge — SuperBPE's phase-1 replay hits this 50 times.

Results (benchmark defaults, before → after)

Case fast (Rust) vs reference
BPE 0.26s → 0.27s (unchanged) 2.0×
BNE n=4 0.54s → 0.49s 2.2×
Boundless BPE 4.16s → 1.40s (3.0×) 3.6×
Super BPE 4.27s → 1.08s (4.0×) 3.9×

Digests unchanged and identical to the reference at 2,000 and 500 samples (20d930a435 / 6598b73bb3 / 9999e69912). Both suites 141 passed, 2 skipped; clippy clean.

Tested and rejected during profiling

  • Python incremental counting for connected multi-doc graphs: digests held but 5.0s → 9.5s (early merges touch nearly every doc; per-doc bookkeeping loses to the memoized C-level recount).
  • Rust dedup/canonicalize and write-back loops: measured 78ms/60µs — left alone.

Ranked next steps (measured, not done)

  1. Incremental seq_recount for connected mode: recount remains the largest piece of what's left; only children adjacent to a merge change.
  2. Token interning to u32 ids: candidate keys are deep-hashed Vec<GraphV>; parallelism hid that cost rather than removing it.
  3. Setup/pretokenization: ~0.1s at this scale — a full-scale-only lever.

🤖 Generated with Claude Code

Phase profiling at the benchmark workload (2000 docs, 100 merges) showed the
connected loop's cost split as apply+recount 2.8s and delta bookkeeping 0.9s,
all serial. Three rayon changes, output byte-identical:

- The affected-entry apply+recount loop runs via par_iter_mut (threshold 64
  entries so small steps skip rayon overhead).
- The old-vs-new candidate diff per entry is computed in parallel; only the
  few real changes are applied serially to the shared global/index (additive
  updates, and pick_best_entries reads index vectors order-independently).
- Trainer.apply_merge parallelizes over Unconn subgraphs with memoized
  try_merge; SuperBPE hits this 50 times replaying phase-1 merges.

Boundless 3.99s -> 1.32s (3.0x), SuperBPE 4.13s -> 0.99s (4.2x),
BNE 0.50s -> 0.46s, BPE unchanged. Digests identical at 2000 and 500
samples, matching the reference on both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmitMY
AmitMY force-pushed the perf/profile-2000 branch from bbfbbb3 to 2495772 Compare July 23, 2026 09:49
@AmitMY AmitMY changed the title perf: profile-driven improvements for both implementations (2000 docs / 100 merges) perf(fast): parallelize per-step apply, recount, and delta diff Jul 23, 2026
@AmitMY
AmitMY merged commit 2229fc7 into main Jul 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant