Skip to content

perf: merge cascade run pairs in parallel (no measurable effect at 12 cores — needs your machine) - #13

Closed
BenjaminDEMAILLE wants to merge 1 commit into
COMBINE-lab:mainfrom
BenjaminDEMAILLE:perf/parallel-cascade
Closed

perf: merge cascade run pairs in parallel (no measurable effect at 12 cores — needs your machine)#13
BenjaminDEMAILLE wants to merge 1 commit into
COMBINE-lab:mainfrom
BenjaminDEMAILLE:perf/parallel-cascade

Conversation

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

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_level walked 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 running src_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 threads partitions 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.md records 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:

  prefetch          2.897  3.606  3.176  2.733  2.837  2.808   mean 3.010
  prefetch+this     3.755  3.368  3.085  2.921  2.747  2.806   mean 3.114

Three pairs each way, means well inside the spread. The same comparison against plain main is 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 main on chr21. 62 tests in debug and release; fmt, Clippy with warnings denied, rustdoc.

🤖 Generated with Claude Code

@rob-p

rob-p commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Thanks for explicitly flagging this as a prediction that needed a 32/64-core measurement. I tested PR head 1c04d1bc42828bbebdfe1617e4faee3dddf7c677 integrated onto current main (cf71aaa75aff9021e9a228f14d52d9f256315733, containing the accepted #9 and #11 work).

Decision: close without merging

The 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.

Reasoning

At cascade level k, the PR parallelizes the independent run pairs at that level. At the final level, however, run_lens.len() == 2, so n_pairs == 1: into_par_iter() receives exactly one job, and that job calls the existing sequential sample_sort::merge over the whole partition. Thus the final full-partition merge remains single-threaded.

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 (4 * 32), so the extra nested jobs mostly add scheduling and synchronization overhead. They do not split the final merge that motivated the change.

Full production-shaped benchmark

Same fixture/protocol as the earlier issue review:

  • GENCODE Human v50 GRCh38 primary-assembly FASTA
  • comprehensive primary-assembly GTF parsed by ruSTAR
  • sjdbOverhang = 100; all 698,597 deduplicated junctions
  • ruSTAR forward/reverse-complement layout, 1,397,582 segments, STAR boundary ordering
  • only A/C/G/T-starting suffixes retained
  • 6,557,611,930 text symbols; 6,176,694,310 emitted suffixes
  • external-memory u64, 8,192 partitions, 32 pinned physical cores

At this input size both variants hit the same 8,192-partition cap, so the PR's separate PHASE1_TARGET_CHUNK change has no effect. This isolates the parallel-cascade change.

One candidate warm-up was followed by an immediate baseline/candidate measurement:

Variant Build (s) Phase 1 (s) Phase 4 (s) User CPU (s) System CPU (s) Peak RSS (KiB) FS output blocks
current main 374.029 105.578 263.242 10,416.68 52.80 10,212,884 193,033,192
#13 394.635 104.538 284.922 10,344.27 70.81 10,960,436 193,033,224
delta +20.606 (+5.51%) -1.040 +21.680 (+8.24%) -72.41 +18.01 +747,552 (+7.32%) +32

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 e81c8f9881e322148741a23c92ae2000.

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

  • focused ruSTAR differential: exact match across 359,616,038 positions
  • full annotated-human count and output hash: exact match
  • 66 debug tests: pass
  • 66 release tests: pass
  • formatting: pass
  • Clippy with warnings denied: pass
  • rustdoc with warnings denied: pass

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.

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.

2 participants