Repository navigation
perf(io): encode SAM/BAM records on the align workers (stacked on #276) - #278
Open
BenjaminDEMAILLE wants to merge 2 commits into
Open
BenjaminDEMAILLE wants to merge 2 commits into
BenjaminDEMAILLE wants to merge 2 commits into
Conversation
…saturated RecordEncoder serializes records to SAM/BAM bytes inside the rayon align stage; the writer thread only appends pre-encoded bytes. Sorted BAM buffers encoded records and sorts an index. Enabled when runThreadN plus a spare margin (4 BAM, 6 SAM) exceeds available cores, since the writer thread is otherwise a free extra core; RUSTAR_PRE_ENCODE=0/1 overrides. BySJout keeps the old path. Output is byte-identical (parity test over 14 configurations). Refs #223 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
19 of 23 tasks
…l-record-encoding Merge of origin/perf/parallel-bgzf-writer (#276: main's external CoordinateSorter + parallel BGZF) into #278 (worker-side SAM/BAM encoding). Conflicts: src/io/bam.rs (7), src/io/sam.rs (1), src/lib.rs (2). sam.rs: kept #276's Default-based BufferedSamRecords::new with #278's encoded fields. bam.rs: CoordinateSorter now buffers BAM-encoded bytes + (ref,pos,off,len) index (replaces #278's SortBuffer and the RecordBuf buffer), spills sorted runs as headerless BGZF of raw records, k-way merges raw records (ties: input order via stable sort + run index). Budget counts real bytes + index entries (estimated_record_bytes removed; its test replaced). SortedBam(Stdout)Writer keep encoder()/write_encoded(). lib.rs: main now builds WithinBAM records on the worker into buf.records, so #278's writer-side supplementary writes and PreEncoder.within_bam were dropped. Verified: fmt, clippy -D warnings, cargo test --release pass; yeast sidx 200k pairs, 8 threads: SAM / BAM Unsorted / Sorted / Sorted+limitBAMsortRAM 1000000 samtools-view bodies md5-identical to a build of origin/perf/parallel-bgzf-writer (with and without RUSTAR_PRE_ENCODE=1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
11 tasks
This branch has not been deployed
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.
Stacked on #276 (base
perf/parallel-bgzf-writer). Refs #223: the follow-up that moves record encoding off the single writer thread.src/io/encode.rs:RecordEncoderproduces SAM/BAM bytes through the same noodles writers the output uses (NoQS stripping, CIGAR check included).write_encoded.SortBufferstores encoded records and stable-sorts an index on (ref id, start), unmapped last, same order as before.runThreadN + spare > available cores(spare 4 for BAM, 6 for SAM). BySJout always keeps the old path.RUSTAR_PRE_ENCODE=1|0forces it.Wall time (s), 16-core Mac, 2M SE 100 bp reads, median of 3:
Identity (
cmpvs main, forced on): SAM, BAM unsorted/sorted,--outStd, NoQS, BySJout, TranscriptomeSAM, GeneCounts, unmapped Fastx, PairedKeepInputOrder, two-pass, PE chimeric WithinBAM, solo: all identical.tests/pre_encode_parity.rscovers 14 configurations on/off.Conflicts: 3-way merge simulation of
src/lib.rsagainst #222 and #261 gives 0 conflicts. Nearest hunks are the solobatch_sizelines; if they conflict, keep both lines. #261 note:AlignmentBatchResultsgrows by 48 bytes, so batches get slightly smaller (no output change).Pre-existing, not fixed here: SE
--chimOutType WithinBAMfails on main ("read length-sequence length mismatch");BufferedSamRecords::new()reserves 10,000 records per read.Tests: fmt, clippy 0 warnings,
cargo test639 passed.🤖 Generated with Claude Code