perf(index): route tandem-repeat-heavy genomes to the in-memory SA path - #262
perf(index): route tandem-repeat-heavy genomes to the in-memory SA path#262BenjaminDEMAILLE wants to merge 1 commit into
Conversation
caps-sa sorts suffixes by comparison, so a comparison costs O(LCP), and a tandem array makes LCPs enormous. The two construction paths degrade very differently there. On a 40 MB genome carrying a 3 MB exact 171-base tandem array — 8% of the genome, roughly what human alpha-satellite looks like — ext-mem takes 50.9s and the in-memory path 27.1s. Both paths produce a byte-identical index (`SA`, `SAindex`, `Genome` and `chrStart.txt` all compare equal on that genome), so the choice between them is purely about speed and cannot change results. Add `use_ext_mem_for`, which keeps ext-mem as the default above the 16 MB threshold but lets a genome take the in-memory path when it is both tandem-repeat heavy and small enough to afford it. `tandem_repeat_score` is the probe: the fraction of sampled positions whose 32-mer recurs at least 8 times within the sample. Separation is wide and the threshold sits in the middle of it. The 3-copy 1%-divergent genome scores 0.0000; the same genome plus the tandem array scores 0.0764. Divergent repeats do not register, because two copies of a locus rarely both land in the sample, while a tandem array collapses its whole span onto a handful of distinct k-mers. Sampling walks a fixed stride, so the decision is deterministic for a given genome, and the probe costs nothing measurable (the non-repetitive build is unchanged at 4.8s). The memory guard is what keeps this honest. In-memory peaks at roughly 68x the text (measured 67.2x and 67.9x on 40 MB and 37 MB inputs) against ext-mem's ~8x, so it is only taken when 72x fits in 40% of RAM. It is conservative when the total cannot be read. A human-scale genome will not fit and stays on ext-mem: the pathology is real there too, but it needs an upstream fix in caps-sa rather than a routing decision here. Also tried and rejected, all measured on the same genome: `CAPS_SA_N_PHYS` at 8 and 64 (51.0s, 51.5s), `CAPS_SA_ORDERED_PHASE4` (50.7s), and `CAPS_SA_SUBPROBLEMS` swept 0 through 4096 (53.7s at best, worse above). The geometric LCP memoization enabled in scverse#228 makes no difference on this input at any setting, including disabled — it never engages here. Measured (Apple M4 Max, 8 threads, wall): genome before after 3 yeast copies at 1% divergence 4.85s 4.79s same, plus a 3 MB tandem array 50.9s 27.1s -47% 599 tests pass (5 new, covering the probe's separation, its degenerate inputs, and the memory guard), 0 clippy warnings, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
I am not convinced a few seconds but more ram usage, for very specific genomes, is worth the fork in algorithms, when capsa could instead be altered to address the problem. It becomes two paths to maintain instead of one, with I would say negligable real worl performance gain, especially on a method that is run once, for a genome, then used multiple times after. Am I missing something? |
|
Upstream context that a reviewer should have, found after opening this PR. The pathology is already known in caps-sa, and a kernel-level fix for it was tried and rejected. COMBINE-lab/caps-sa#10 ("skip long periodic runs instead of scanning them") inferred That matters for how this PR is shaped. The objection to #10 was cost on the common path, and this change does not have that shape: the probe runs once per index build, never per comparison, which is why the non-repetitive build is unchanged (4.85s → 4.79s). It sidesteps the problem at the call site rather than paying for it in the kernel. The same reasoning explains the negative result I reported above about the geometric LCP memo from #228 — a per-lookup table is the thing that does not pay here, whether it lives in the kernel or in the memo. I posted this PR's measurements to the upstream tracking issue as an independent workload (COMBINE-lab/caps-sa#7, comment 5461685963): the tandem-array case is distinct from the chr21 period-61 Version note, relevant to whether this stays neededcaps-sa 0.7.0 — what we pin — was published 2026-08-13. Upstream #11 and #12 are in it; #14 and #15 merged on 2026-08-14 and are not released yet. #15 is "optionally seed phase 1 with segment-aware packed keys", and the segmented path is exactly the one ruSTAR uses. So the honest position on this PR: it is worth having now, and it should be re-validated when we next bump caps-sa. If a released version makes the tandem case fast on ext-mem, this routing becomes dead weight and should be removed rather than left in. I have offered upstream to re-run the measurements against |
|
@Psy-Fer You are not missing much, and I think you are right. Let me answer with the measurement I ran after your comment rather than an argument, because it settles the main question. "caps-sa could instead be altered to address the problem." I built ruSTAR against caps-sa
So the pathology is untouched by the newest upstream work. Worth knowing, but it does not rescue this PR. The point that I think actually decides it, and it is against me. The memory guard requires ~72x the text to fit in 40% of RAM. Human is ~3.1 Gb, needs ~210 GB, and therefore never qualifies — it stays on ext-mem and keeps paying in full. But human alpha-satellite is exactly what I used to motivate the change. So this PR cannot help the case I justified it with. What is left is small-to-medium repeat-heavy genomes on well-provisioned machines, and I have no real genome demonstrating that win — only a synthetic 40 MB construction. Against a once-per-genome operation, that is a thin case, which is your point. One factual correction, though it does not change the conclusion. This does not fork the algorithms: both paths already exist in On "alter caps-sa instead" being the better fix: agreed in principle, and it has been attempted. Upstream COMBINE-lab/caps-sa#10 did exactly that — inferring My recommendation: close this. The measurements are the durable part and they are already on the upstream tracking issue (COMBINE-lab/caps-sa#7) where they can inform a kernel fix. I would rather withdraw a marginal heuristic than have you maintain it for a case I cannot demonstrate on a real genome. If you would rather keep something, the version I would defend is much smaller: no probe, no automatic routing, just documenting in The other three perf PRs (#256, #257, #261) are unaffected by this and stand on their own. |
Independent of #256 / #257 / #261 — touches only
src/index/sa_build.rs(plus a directlibcdep already in the tree) and can merge in any order.The problem
caps-sa sorts suffixes by comparison, so a comparison costs O(LCP), and a tandem array makes LCPs enormous. Measured on a 40 MB genome, 8 threads:
A 171-base monomer array is what human alpha-satellite looks like, and centromeres carry tens of Mb of it, so this is not only a synthetic worst case.
Why routing is the fix
The two construction paths degrade completely differently on this input:
And critically: both paths produce a byte-identical index.
SA,SAindex,GenomeandchrStart.txtall compare equal on the satellite genome. So the choice is purely about speed and cannot change results — which is what makes a heuristic acceptable here at all. The worst a wrong guess can do is pick the slower path.use_ext_mem_forkeeps ext-mem as the default above the existing 16 MB threshold, and lets a genome take the in-memory path when it is both tandem-repeat heavy and small enough to afford it.The probe
tandem_repeat_scoreis the fraction of sampled positions whose 32-mer recurs ≥8 times within the sample. Separation is wide, and the threshold (0.02) sits in the middle of it:Divergent repeats deliberately do not register — two copies of a locus rarely both land in the sample — while a tandem array collapses its whole span onto a handful of distinct k-mers. Sampling walks a fixed stride, so the decision is deterministic for a given genome, and the probe costs nothing measurable: the non-repetitive build is unchanged (4.85s → 4.79s).
The memory guard
This is what keeps it honest. In-memory peaks at roughly 68x the text (measured 67.2x and 67.9x on 40 MB and 37 MB inputs) against ext-mem's ~8x, so the in-memory path is only taken when 72x fits inside 40% of RAM, and the guard is conservative when the total cannot be read.
A human-scale genome will not fit and stays on ext-mem. I want to be straight about that: the pathology is real at human scale too, and this PR does not fix it there. Fixing it there needs an upstream change in caps-sa — its phase-4 geometric LCP memoization never engages on this input, so the cost is in the sample sort itself.
Rejected alternatives, all measured on the same genome
CAPS_SA_N_PHYS=8/=64CAPS_SA_ORDERED_PHASE4=1CAPS_SA_SUBPROBLEMS0→4096Result
N), and the memory guard--all-targets --release)cargo fmt --checkclean🤖 Generated with Claude Code