Skip to content

Tracking: performance PR series (#137, #138, #139, #140) and reproducibility fix (#142) #143

Description

@BenjaminDEMAILLE

Tracking issue for a batch of five PRs: four performance changes plus one reproducibility fix found while verifying them. Each was measured before being opened; one candidate was implemented, measured, and dropped because it showed no gain.

The PRs

PR Change Measured effect Files touched Risk
#139 Wrap the 28 unbuffered output writers in BufWriter 2.4x wall clock (23.16 s → 9.72 s), sys time -95% 14 src files Low — no logic change
#138 flate2zlib-rs backend 2.32x gzip decompression (777 ms → 335 ms) Cargo.toml only Low — dependency swap
#140 GTF parsing without per-line allocations 1.88x (2.373 s → 1.264 s on the human GTF) src/gtf.rs Medium — parser rewrite, edge cases covered by new tests
#137 mimalloc as #[global_allocator] (binary only) 1.06-1.08x wall clock, user CPU -15% Cargo.toml, src/main.rs Low — behind a default-on feature
#142 Deterministic tie-breaks (fixes #141) Correctness, not speed 3 src files Low

Combined effect

I merged all four perf branches locally and measured the stack (4 interleaved runs against main, medians):

real user sys
main 23.28 s 23.98 s 13.43 s
all four 8.75 s 20.09 s 0.59 s

2.66x wall clock. The gains are roughly additive because they hit different resources: #139 removes syscalls, #137 removes allocator contention, #138/#140 speed up the annotation-loading phase.

Every data output was byte-identical between the combined stack and mainfeatureCounts.tsv, dupMatrix.txt, tin.xls, lc_extrap.txt, samtools stats/flagstat, read_distribution.txt, junction.xls, inner_distance.txt. The one file that differed, qualimap/rnaseq_qc_results.txt, is the pre-existing nondeterminism in #141, which #142 fixes.

Benchmark setup

Everything above was measured on the same two inputs. Neither is real sequencing data — see the caveats.

BAM — 4,000,000 reads (2M pairs), coordinate-sorted and indexed, 24 contigs of 40-58 Mb, 15% duplicate-flagged, 25% spliced with realistic N CIGAR operators, random 100 bp sequences (this matters: an early version used a constant sequence, which left the read_duplication maps with one entry and silently invalidated part of the measurement).

GTF — two annotations were used depending on what was being measured:

Protocol. The machine turned out to be noisy (hyperfine reported ±5 s swings on some runs), so whole-pipeline numbers come from interleaved A/B rounds reporting medians of real/user/sys, not from consecutive hyperfine blocks. Isolated measurements (GTF parse, gzip inflate) are hyperfine -w 1 -r 5 and are stable to ±1%.

Machine: aarch64 macOS (Darwin 25.6.0), rustc 1.97.1, --threads 4, cargo build --release (LTO, codegen-units = 1).

Correctness protocol for each PR: cargo test --release, cargo clippy -- -D warnings, cargo fmt --check, plus a full-output byte-comparison against main on the 4M-read dataset, normalising the output-directory path that some file headers embed.

Where the time actually goes

Worth recording, because it redirected this work. A sampling profile of main on the benchmark input showed the top non-idle frame was write in libsystem_kernel — the process was spending more time in the write syscall than in any analysis code (sys 13.4 s against real 23.3 s). That is what #139 addresses, and it is why #139 rather than any of the algorithmic ideas is the largest win here.

Rejected: rustc-hash / FxHashMap

The obvious candidate — swap SipHash for FxHasher on the hot integer-keyed maps — was fully implemented (12 maps across accumulators.rs, bam_stat.rs, stats.rs, plus the paired-end mate buffer in counting.rs) and then dropped.

Interleaved A/B, 5 rounds, medians: main 23.65 s vs FxHash 23.62 s. No measurable difference, so no PR. The profile above explains it: at the time of measurement the pipeline was syscall-bound, not hash-bound.

It may be worth re-testing after #139 lands, since removing 13 s of syscall time changes what the bottleneck is. If someone wants to pick this up, note that tin.rs deliberately keeps its own seeded BuildHasher (TinHashState) because the HashSet layout feeds a threshold-crossing check during parallel accumulation — that one must not be swapped.

Merge order

#137 and #138 both edit the dependency section of Cargo.toml and the dependency table in AGENTS.md; merging both produced one trivial conflict in AGENTS.md (two rows in the same table). #139 and #142 both touch dupradar/plots.rs, qualimap/output.rs, and qualimap/report.rs, but in different functions — they merged cleanly for me. Suggested order, cheapest to review first:

  1. perf: switch flate2 to the zlib-rs backend (2.3x faster gzip) #138 (dependency swap, one line)
  2. perf: use mimalloc as the global allocator in the binary #137 (allocator, two files)
  3. fix: make dupRadar and Qualimap outputs reproducible run-to-run #142 (correctness fix, closes Three outputs are not reproducible run-to-run (HashMap iteration order reaching output) #141)
  4. perf: buffer all output file writers (2.4x wall clock) #139 (mechanical, 14 files, biggest win)
  5. perf: parse GTF without per-line allocations (1.88x) #140 (parser rewrite, needs the closest review)

Caveats

  • Synthetic data. The BAM is generated, not sequenced. It exercises the code paths (mate pairing, junctions, duplicate marking, per-gene counting) but its coverage distribution is not biologically realistic. Numbers on real data from the RustQC-benchmarks pipeline would be more authoritative, and I would expect perf: buffer all output file writers (2.4x wall clock) #139 to look better there, since real datasets have far more read pairs and therefore far more lines in the inner_distance detail file.
  • One machine, one architecture. aarch64 macOS only. The zlib-rs comparison in particular could look different on x86-64, where the SIMD paths differ.
  • perf: use mimalloc as the global allocator in the binary #137's benefit is contention-related and was measured at --threads 4. It should grow with thread count and I have not measured that curve.

Happy to re-run any of this against a real dataset, or to split #139 if 14 files in one PR is too much to review at once.


🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions