Repository navigation
Five seed-search and index speedups from #536, and a debug-check fix - #542
Merged
Merged
Conversation
AlignmentCache::addOnTheFlyImpl added one OFF_MASK entry per element of a BW range with a separate PList::add call each. The new PList::addFillUpTo stores the same entries with std::fill_n, one page-sized run at a time, and takes pages from the pool exactly when add() would, so a pool that runs out leaves the list, the pool and payload.len in the same state as before. From PR #536 (item 8), found by @jmschrei's auto-optimize run. Co-authored-by: Benjamin Langmead <langmea@cs.jhu.edu>
The BWT, ftab, eftab and SA sample arrays and the bit-packed reference are now allocated with 64 extra bytes and used from the first 64-byte boundary (APtrWrap::initAligned, mmAlignUp), and freed through the pointer new[] returned. glibc serves large new[] blocks with mmap at an address 16 bytes into a page, so every 64-byte BWT side straddled two cache lines and each LF step touched two lines instead of one. Only where the arrays start in memory changes; --mm and --shmem keep using the mapped or shared memory as before. This also removes the misaligned-load reports ASan gave in Ebwt::countUpTo on master. From PR #536 (item 9, first part), found by @jmschrei's auto-optimize run. Co-authored-by: Benjamin Langmead <langmea@cs.jhu.edu>
Ebwt::joinedToTextOff found the reference fragment holding an offset with a binary search over all fragment start positions. A table built at index load time (buildFragTab, at most 2^15 + 1 entries, about 91 KB for hg38) now gives, for each bucket of the joined text, the fragment holding the bucket's first offset, so the binary search is confined to the fragments between one bucket entry and the next, usually just one. The old full search remains as the fallback when the table is absent (for example for the mirror index, which has no rstarts) or the offset lies beyond it. The straddle and offset computation after the search is unchanged, moved out of the loop so both paths share it. The table is read only after loading, so it needs no locking. From PR #536 (item 30), found by @jmschrei's auto-optimize run. Co-authored-by: Benjamin Langmead <langmea@cs.jhu.edu>
SwDriver::extend extended an exact seed hit first to the left, LF-stepping in the forward index, and then to the right in the mirror index. The two extensions are independent chains of dependent memory loads, so they are now kept as two ExtChain states and stepped alternately (extInit, extStep), each step ending with a prefetch of that chain's next BWT side, so one chain's load is in flight while the other steps. Each chain stops under the same conditions as before (a mismatch, more than one possible character, a shrinking range, 255 positions, or the end of the read), and nlex, nrex and the per-read FM-op count come out the same. The debug-only copy of the extended read into tmp_rdseq_ is dropped; tmp_rdseq_ is only scratch space for matchesRef(). From PR #536 (item 29), found by @jmschrei's auto-optimize run. Co-authored-by: Benjamin Langmead <langmea@cs.jhu.edu>
SeedAligner::searchAllSeeds searched the forward-strand seed offsets in batches of 8, committing each batch's results, and then did the same for the reverse-complement offsets, allocating its caches and scratch vectors on every call. It now takes one SeedSearchCache per seed offset of both strands first, runs the searches in batches of up to SEED_BATCH_CACHES (32) offsets with both strands mixed, so more FM-index walks are in flight together, and then hands the results to the alignment cache and to SeedResults in the old order, forward offsets first. The searches write only their own caches and the bwops_/bwedits_ counts, so the results are the same. The caches (SeedSearchMultiCache, whose SeedSearchCache now holds pointers to the seed and can be reset) and the search scratch are members of SeedAligner, kept across calls, so they allocate only when a read needs more seed searches than any earlier one; SeedAligner is therefore no longer copyable. From PR #536 (item 27), found by @jmschrei's auto-optimize run. Co-authored-by: Benjamin Langmead <langmea@cs.jhu.edu>
In debug builds, SwAligner::nextAlignment reruns the checkpointed backtracer after the striped 16-bit local backtrace and asserted that the first alignment found by the two was identical. Both start from the same cell, but where two alignments tie they can choose differently at the other end: with --local on example/reads/longreads.fq, read r479 aligns as 7M3D742M (the seven matches' bonus cancelled by the gap) or with those seven bases soft-clipped, both scoring 1440, and the debug binary aborted. The check now requires what holds with ties: the same success, the same score, and the same right end on the reference and in the read. The 8-bit branch already has its equivalent assertion commented out for the same reason. Release builds and SAM output are unchanged.
This was referenced Oct 24, 2026
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.
This PR takes five self-contained changes from @jmschrei's auto-optimize PR #536, one commit each, so they can be reviewed and reverted separately. Each commit contains only that item's hunks from #536. Every changed line comes from #536's diff, and nothing from its other items is included. A sixth commit relaxes a debug-only assertion that aborts on master.
PList::addFillUpTofills the seed-hit offset list a page at a time instead of callingadd()once per element (ds.h,aligner_cache.cpp).mm.h,ds.h,bt2_io.cpp,reference.*). A 64-byte BWT side then touches one cache line instead of two. This does not apply with--mmor--shmem. It also removes the misaligned-load reports ASan gives on master inEbwt::countUpTo.joinedToTextOfffinds the reference fragment through a bucket table built when the index loads, with the old binary search kept as the fallback (bt2_idx.*,bt2_io.cpp). The table is at most 128 KB, about 91 KB for hg38.SwDriver::extendsteps its left and right extensions alternately, so their memory loads overlap (aligner_sw_driver.*).searchAllSeedssearches the seed offsets of both strands in shared batches of up to 32, keeps its caches and scratch space across reads, and commits results in the old order (aligner_seed.*). As a result,SeedAlignercan no longer be copied.SwAligner::nextAlignment; see below.The debug-check fix.
-x example/index/lambda_virus --local -U example/reads/longreads.fq, read r479 can align two ways, both scoring 1440. One is7M3D742M, where the 3-base gap cancels the bonus from the seven matches. The other soft-clips those seven bases. The debug binary aborts on this read.Output: unchanged. The release-build SAM was byte-identical to master in every case below, ignoring the
@PGline.--local, paired and unpaired,-k 5,-a,--very-sensitive,--very-fast-local,-p 4 --reorder,--mm.--local,-k 5,--very-sensitive-local,--very-fast-local,-a,--ma 3 --mp 4,-p 4 --reorder.--local,-k 10,-p 4 --reorder.-p 1and-p 16.The debug build gave the same output as master's release build on the lambda cases. It also runs the long-read cases to completion; master's debug build aborts on them.
Tests:
make allallsucceeds at the branch head./proctest.Speed. Intel Xeon Gold 6448Y, GRCh38 no-alt index, 250,000 read pairs, median of 3 runs. The nodes were shared, so expect run-to-run noise of about 1% to 5%. Speedups are master's time divided by this branch's.
-v256) build-p 1-p 16 --reorder-p 1/-p 16-k 200)-p 1On an Apple M4 Pro with E. coli (200,000 pairs), the gain was 1.04x end-to-end and 1.05x with
--local.Most of the WGS gain comes from the first commit (item 8), which alone gives 1.032x to 1.037x. Each of the other four is within noise when added one at a time, though none showed a consistent slowdown. They are included because they are low-risk and because they keep #536's later items applying cleanly. Their share of the gain should grow once #536's DP changes land, since seed search and index lookups then take up more of the run time.
Credit. The five speedups are @jmschrei's work, found by his auto-optimize run in #536. He is the author of those five commits. This particular PR, drawing on elements of 536, was split out and prepared by Ben Langmead with help from Claude Opus 5.5.