Skip to content

perf(index): route tandem-repeat-heavy genomes to the in-memory SA path - #262

Open
BenjaminDEMAILLE wants to merge 1 commit into
scverse:mainfrom
BenjaminDEMAILLE:perf/sa-path-routing
Open

perf(index): route tandem-repeat-heavy genomes to the in-memory SA path#262
BenjaminDEMAILLE wants to merge 1 commit into
scverse:mainfrom
BenjaminDEMAILLE:perf/sa-path-routing

Conversation

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

Independent of #256 / #257 / #261 — touches only src/index/sa_build.rs (plus a direct libc dep 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:

genome build
3 yeast copies at 1% divergence 4.85s
same, plus a 3 MB exact 171-base tandem array (8% of the genome) 50.9s

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:

genome ext-mem in-memory
normal 4.81s / 294 MB 6.71s / 2401 MB
+ tandem array 50.9s / 305 MB 27.1s / 2572 MB

And critically: both paths produce a byte-identical index. SA, SAindex, Genome and chrStart.txt all 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_for keeps 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_score is 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:

  • 3-copy, 1%-divergent genome: 0.0000
  • same genome + tandem array: 0.0764

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

knob result
CAPS_SA_N_PHYS=8 / =64 51.0s / 51.5s
CAPS_SA_ORDERED_PHASE4=1 50.7s
CAPS_SA_SUBPROBLEMS 0→4096 53.7s at best, worse above
geometric LCP memo (#228), any setting incl. disabled no difference — never engages here

Result

genome before after
3 yeast copies at 1% divergence 4.85s 4.79s
same, plus 3 MB tandem array 50.9s 27.1s (-47%)
  • 599 tests pass — 5 new, covering the probe's separation, its degenerate inputs (empty, shorter than k, all-N), and the memory guard
  • 0 clippy warnings (--all-targets --release)
  • cargo fmt --check clean

🤖 Generated with Claude Code

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>
@Psy-Fer

Psy-Fer commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

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?

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor Author

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 lcp(a, b) >= e - b in O(1) from a run's bounds instead of scanning it. It was closed by the maintainer: consulting a run table taxed the common path on real annotated-human input, even with lazy lookup gated behind 256 ordinary symbols.

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 N-wrapping case that issue was opened about, and the in-memory path absorbing the same input 1.9x better than ext-mem may be a useful hint about where the cost actually sits.

Version note, relevant to whether this stays needed

caps-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 main if that would help them size #15's effect. Worth a TODO referencing the upstream issue if you would rather not rely on someone remembering — say the word and I will add one.

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor Author

@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 main (d76ba9d, which has the just-merged upstream #14 and #15#15 is segment-aware phase-1 seeding, and the segmented path is the one we use) to see whether the upstream fix is already here. It is not:

genome caps-sa 0.7.0 (pinned) caps-sa main
normal, ext-mem 4.81s 5.14s
+ tandem array, ext-mem 51.4s 52.0s
+ tandem array, in-memory 27.1s 28.99s

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 sa_build.rs and are already selected automatically by a 16 MB threshold plus the RUSTAR_USE_IN_MEM / RUSTAR_USE_EXT_MEM overrides. The PR changes which path gets chosen, not how many exist, and both produce a byte-identical index. But it does add a probe, a threshold and a RAM-budget check (including a platform-specific sysctl) to maintain, and that is a real cost you are right to weigh.

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 lcp(a, b) >= e - b in O(1) from a periodic run's bounds instead of scanning — and was closed by the maintainer because consulting the run table taxed the common path on real annotated-human input, even with lookups gated behind 256 ordinary symbols. The same shape of result shows up in the geometric LCP memo we enabled in #228: it makes no difference on this input at any setting, including disabled. So the kernel fix is the right target but is not sitting there waiting to be taken.

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 sa_build.rs that RUSTAR_USE_IN_MEM=1 is roughly 2x faster on tandem-repeat-heavy genomes that fit in RAM, so anyone hitting it has an escape hatch. That is a comment, not a code path. Happy to do that instead, or to close outright — your call.

The other three perf PRs (#256, #257, #261) are unaffected by this and stand on their own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants