format SAM/PAF in the mapping step instead of the output thread - #33
Open
nh13 wants to merge 2 commits into
Open
format SAM/PAF in the mapping step instead of the output thread#33nh13 wants to merge 2 commits into
nh13 wants to merge 2 commits into
Conversation
At high thread counts the single output thread (step 2) is the wall: it calls mb_format() for every read and fwrite()s serially, so once mapping is fast enough the mapping threads finish and idle while one thread formats and writes all the text. Build it in a parallel kt_for() over superbatches at the end of step 1 -- each worker fills its own kstring from a per-thread kalloc pool -- and let step 2 just fwrite() the prebuilt buffers in input order. Output is byte-identical and in the same order.
The format buffer grew from zero by powers of two, so on a full chunk it reallocated ~7 times per superbatch and left ~1.7x the final size in rounded-up scratch (measured 260 MB allocated for 153 MB of text). Reserve the buffer up front from the superbatch's bases (2*l_seq for SEQ+QUAL plus a per-record allowance); allocated scratch drops to ~171 MB and the realloc chain is gone. Output is unchanged.
Collaborator
Author
|
For context: #32 is the complementary half of this work -- it makes the per-record formatter ( |
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.
Mapping output is currently formatted serially in step 2: the single output thread calls
mb_format()for every read andfwrite()s the text. Once mapping is fast (many threads, short reads), the mapping threads finish and idle while that one thread formats and writes everything, so the output thread becomes the wall.This builds the SAM/PAF text in step 1 instead, in a
kt_for()over superbatches. Each worker formats its superbatch into its ownkstring_tfrom a per-thread kalloc pool, so the pass is lock-free and adds no per-superbatch allocator churn. Step 2 then justfwrite()s the prebuilt buffers in input order. Output is byte-identical and in the same order as before; I checked this on paired-end, interleaved, and single-thread runs.The effect is confined to the high-thread regime, where the output thread is the bottleneck. On a 96-vCPU Graviton4 (c8g.24xlarge), gcc 15, aligning about 20M paired-end reads (2x150 bp) against GRCh38, median of 5 runs:
effis parallel efficiency relative to the 32-thread point,(wall_32 / wall_N) * (32 / N), so 1.00 is ideal scaling from 32 threads. On master, efficiency falls to 0.74 at 96 threads because the output thread cannot keep up; with this change 96 threads run at 0.90. Below ~64 threads the output thread is no longer the limit and the two are within run-to-run noise. This matters for whole-genome runs on large many-core machines, where it shows up as wall time (and, on spot instances, exposure to preemption) rather than throughput per core.The second commit pre-sizes each superbatch's output buffer from its base count instead of growing it from zero. The geometric growth left about 1.7x the final text in rounded-up scratch (260 MB allocated for 153 MB of text on a default
-Kchunk); reserving up front drops that to ~171 MB and removes the realloc chain. Working-set memory is otherwise unchanged -- the buffered text is one chunk's worth, bounded by-K, not the whole file -- and total peak RSS is index-dominated and does not move measurably.This is independent of #32, which makes the per-record formatter (
mb_format) itself cheaper; the two touch different files and compose.If a per-stage timing breakdown (read/decompress, mapping, formatting, write) would be more convincing than the efficiency curve, I can add one -- it shows the format+write time dropping out of the serial tail directly.