perf: merge cascade run pairs in parallel (no measurable effect at 12 cores — needs your machine) - #13
Conversation
|
Thanks for explicitly flagging this as a prediction that needed a 32/64-core measurement. I tested PR head Decision: close without mergingThe transformation is correctness-preserving, but it does not address the stated tail bottleneck and it causes a clear slowdown on the real annotated-human ruSTAR workload. ReasoningAt cascade level The added nested Rayon scheduling can only expose parallelism in earlier levels, when each partition has multiple pairs. Those are precisely the levels where phase 4 already has abundant outer parallelism across 128 in-flight partitions ( Full production-shaped benchmarkSame fixture/protocol as the earlier issue review:
At this input size both variants hit the same 8,192-partition cap, so the PR's separate One candidate warm-up was followed by an immediate baseline/candidate measurement:
The candidate warm-up was nearly identical: 394.306 s total, 105.988 s phase 1, and 283.216 s phase 4. The two candidate totals differ by only 0.329 s. The two nearby baseline totals were 373.800 and 374.029 s. This is therefore decisively outside noise, and the regression is localized to the phase the PR changes. All full runs emitted exactly 6,176,694,310 positions with stream hash On the focused chr21-backbone + full-annotation-flank fixture, #13 also regressed from 15.502 to 16.406 s (+5.83%) while matching all 359,616,038 output positions. That smaller result mixes the cascade change with the altered default partition count, so I am not using it as the primary decision evidence; the capped full-genome result above is the clean comparison. Validation
I am closing #13 without merging, consistent with the criterion in the PR description. To attack the actual tail, a future design would need to parallelize/split a single merge (or change the outer scheduling so large final merges overlap better), not merely parallelize the multiple pairs that exist before the final level. |
Independent of every other open PR. Branches from
main.I cannot demonstrate a speedup for this on my hardware, and I am opening it anyway because the rationale rests on numbers from yours. @rob-p, this needs measuring at 32 or 64 cores before it should be believed.
What
CascadeWorkspace::merge_one_levelwalked its run pairs sequentially, although the pairs at a level are independent and write to disjoint destination ranges. The only thing forcing the order was the runningsrc_off/dst_off, and both are prefix sums, so they can be computed up front and each pair handed its own sub-slices.Why it should matter
Partition-level parallelism (
4 x threadspartitions in flight) hides the serial cascade while there are many partitions still running, but each partition's cascade ends in a single 2-way merge over the whole partition, and those tails serialise.bench/README.mdrecords phase 4 at 52% parallel efficiency on GRCh38 with 32 threads, lifted to 72% by the chunk-size change — the remaining 28% is exactly this shape.Why I cannot show it
At 12 threads with
p = 612, there are ~51 partitions per thread, which is more than enough to hide every tail. Interleaved against the prefetch branch (#12) as baseline, six pairs:Three pairs each way, means well inside the spread. The same comparison against plain
mainis equally flat (4.078 against 4.089).An earlier reading of 2.11 s against 2.02 s on a larger stack looked like a 4% win; replicating it properly showed it was noise. Recording that here rather than quoting it.
So this is a correctness-preserving parallelism fix whose benefit is a prediction, not a measurement. If it does not measure on a 32- or 64-core run either, it should be closed — the code is more complex than what it replaces and complexity without evidence is not worth carrying.
Verified
Output byte-identical to
mainon chr21. 62 tests in debug and release; fmt, Clippy with warnings denied, rustdoc.🤖 Generated with Claude Code