Fix one-gap adapter search offset (#728) and speed up matchWithOneInsertion

[dougnukem]

Fixes #728.

trimBySequence's one-gap search compared the adapter with the start of the read at every position, instead of at the position being tested. So it missed adapters with a one-base indel, and it trimmed the tail of reads that only start like the adapter (~1.8% of reads on a public WGBS run). Details and a 5-read reproduction are in #728.

Changes

1. fix: pass rdata + pos to both Matcher::matchWithOneInsertion calls in AdapterTrimmer::trimBySequence, the same offset the exact-match loop already uses.
2. perf: early-exit Matcher::matchWithOneInsertion.
   • Why: with (1) the search does the comparisons it was meant to do, and it's already the largest function in the worker threads (33% of worker CPU in a perf profile of SRR10007843).
   • Old version: built the full prefix and suffix mismatch arrays on every call, including an O(length) sentinel fill.
   • New version: scans the prefix forward and the suffix backward, stops each scan once it can't be part of a match within diffLimit, then checks only the splits both scans reached. On unrelated sequence both stop within a few bases.
   • Same results: the previous implementation is kept as matchWithOneInsertionReference. It's used for cmplen > 512 and by the new Matcher::test.
3. tests:
   • AdapterTrimmer::test gains a TruSeq adapter at position 40 with a one-base insertion and a one-base deletion (both fail on master), and a read that starts like the adapter (master cuts it from 81 to 72 bp).
   • Matcher::test compares the new and reference matchWithOneInsertion on 200k random, near-match and exact cases.

Verification

• fastp test passes. Each new test fails without the change it covers.
• Matcher::test catches deliberate off-by-one mutations of the new function (4 of 4 tried).
• Full-size output of "fix only" and "fix + early-exit" is byte-identical on all three runs below. The early-exit change is a pure speed change.

Performance

Complete public runs, fastp -w 16 (PE with --detect_adapter_for_pe), GCE n2d-highmem-48, inputs in page cache. Mean of 2 runs in alternating build order.

run	1.3.7 wall / CPU	fix only	fix + early-exit (this PR)
SRR10007843 PE 150	59.5 s / 827 CPU-s	79.9 s (+34%) / 1171 (+42%)	67.4 s (+13%) / 954 (+15%)
ERR10308506 WGBS PE	69.6 s / 578 CPU-s	72.4 s (+4%) / 647 (+12%)	68.1 s (−2%) / 561 (−3%)
ERR10669429 SE 75	38.8 s / 333 CPU-s	40.7 s (+5%) / 427 (+28%)	39.7 s (+2%) / 357 (+7%)

A separate benchmark on GitHub-hosted 4-CPU runners, which projects 300K–500K-pair subsets to a 50M-read run (synthetic PE/SE 150 bp and SRR891268 ATAC PE 50, -w 1 and -w 4), gave:

build	projected wall vs master	projected CPU vs master
fix only	+11% to +45%	+12% to +30%
fix + early-exit	−9% to +12%	−9% to +8%

Peak RSS is unchanged.

So the fix makes trimming correct at roughly the old cost. It isn't a speedup, since the search now does the work it was meant to do. matchWithOneInsertion is still the largest worker function after this change. A bit-parallel or SIMD version of the one-gap search could take it further, but would change results in edge cases, so it's left out of this PR.

Output changes users will see

• More adapter-trimmed reads on libraries with indel-bearing adapters (+0.5% to +0.6% on the RNA-seq runs above).
• Fewer false trims on libraries whose reads often start like the adapter. The WGBS run above trims 15% fewer reads, because fastp detects a T-run-prefixed adapter there and bisulfite reads are T-rich.

🤖 Generated with Claude Code