perf(io): encode SAM/BAM records on the align workers (stacked on #276) - #278
Open
BenjaminDEMAILLE wants to merge 1 commit into
Open
BenjaminDEMAILLE wants to merge 1 commit into
BenjaminDEMAILLE wants to merge 1 commit 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>
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
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