Skip to content

feat: scale line index to gigabase single contigs#18

Open
Istar-Eldritch wants to merge 2 commits into
mainfrom
feat/large-contig-line-index-memory-scaling
Open

feat: scale line index to gigabase single contigs#18
Istar-Eldritch wants to merge 2 commits into
mainfrom
feat/large-contig-line-index-memory-scaling

Conversation

@Istar-Eldritch

Copy link
Copy Markdown
Contributor

Problem

SequenceIndex retained one LineEntry per physical FASTA line for the lifetime of the reader — roughly 1 byte of JVM heap per base. A single gigabase-scale contig (e.g. individual wheat/axolotl/lungfish chromosomes >1 Gbp) could therefore exhaust multiple GB of heap and OutOfMemoryError before any sequence was read, regardless of how memory-efficient the consumer was. Full write-up in docs/2607081600_large_contig_line_index_memory_scaling.md (included in this branch).

Approach

Introduce a LineIndex abstraction with a piecewise-regular representation. A record is stored as a list of regular segments — each a run of lines sharing one base width W and one byte stride S, with only the run's final line allowed to be shorter. Byte offsets are computed arithmetically within a segment, so memory scales with the number of irregularities, not the number of lines.

  • RegularLineIndex — a uniformly wrapped record (the norm, including gigabase contigs) collapses to a single segment stored as four longs. The 2.5 Gbp case goes from ~2.3 GB to ~48 bytes for the line index.
  • SegmentedLineIndex — records with blank lines, width changes, or mixed separators use a handful of segments; lookup binary-searches the (few) segments then computes arithmetically.
  • LineSegmenter — the builder now segments lines in a single streaming pass, retaining only closed segments plus the open one, so index construction is bounded-memory for every input (no more per-line list, no two-pass fallback).

Compatibility

  • Public API (FastaReader / SequenceReader / SequenceFormatReader) and byte-span semantics are unchanged.
  • Breaking change to persisted index JSON: the per-line lines array is replaced by a polymorphic lineIndex (kind: regular | segmented), which also keeps the on-disk index compact. Warrants a version bump.

Tests

  • LineIndexTest — segmentation verified against a per-line source of truth (uniform → regular, short final line → still regular, stride/width changes → segmented, single-line, empty) plus JSON round-trip for both representations.
  • SequenceIndexBuilderTest — builder picks the right representation (short-tail/CRLF stay regular; blank line / short middle line → segmented) and byte offsets match per-line lookup.
  • Full suite green.

The per-line SequenceIndex retained one LineEntry per physical FASTA line
for the lifetime of the reader (~1 byte of heap per base), so a single
gigabase-scale contig could exhaust multiple GB of heap before any sequence
was read.

Replace the per-line list with a LineIndex abstraction and a piecewise-regular
representation: a record is stored as regular segments, each a run of lines
sharing one base width and byte stride (only the final line of a run may be
shorter). Byte offsets are computed arithmetically within a segment, so memory
scales with the number of irregularities rather than the number of lines. A
uniformly wrapped record collapses to a single segment (RegularLineIndex, four
longs); irregular records use SegmentedLineIndex.

The builder now segments lines in a single streaming pass (LineSegmenter),
retaining only closed segments plus the open one, so index construction is
bounded-memory for every input. Persisted index JSON changes accordingly
(polymorphic kind: regular/segmented) and stays compact on disk.
@Istar-Eldritch

Copy link
Copy Markdown
Contributor Author

Validated against the downstream consumer that surfaced this issue (gff3tools)

This fix was validated end-to-end against gff3tools (uk.ac.ebi.ena:fastareader consumer), the project whose CLI conversion tool originally surfaced the OOM — see enasequence/gff3tools#148 and docs/2607081600_large_contig_line_index_memory_scaling.md on this branch for the original repro.

Methodology

Published this branch to mavenLocal (./gradlew publishToMavenLocal, version bumped to a scratch identifier for testing), pointed a local gff3tools checkout at it, rebuilt (--offline), and re-ran the same 2.5 Gbp single-contig FASTA + GFF3 stress fixture that originally triggered the OOM — sweeping -Xmx and sampling VmHWM from /proc/<pid>/status throughout each run.

Result

Before this fix After this fix
2.5 Gbp single contig, -Xmx2G OutOfMemoryError — crash confirmed (via live monitoring + output file size) to happen before any output is written succeeds
Minimum heap to convert 2.5 Gbp fails even at 2G succeeds down to -Xmx96m, peak RSS ~273 MB
2.5 Gbp conversion, -Xmx768m OOM succeeds, ~59s, output byte-correct (2500000000 BP ID line, correct SQ base counts, clean // terminator)

A jcmd <pid> GC.class_histogram snapshot before the fix showed LineEntry/GapRegion dominating live memory (11M+ / 15M+ instances for a ~41.6M-line record); after the fix, the same fixture never approaches heap pressure regardless of -Xmx, consistent with the RegularLineIndex fast path collapsing the line index to O(1) for this uniformly-wrapped record.

Consumer compatibility

Confirmed this is a safe, non-breaking internal change from the consumer's point of view:

  • gff3tools never references SequenceIndex, LineEntry, or getSequenceIndex() anywhere (grepped its full source tree) — it only consumes the stable SequenceFormatReader surface (getSequenceSlice, getSequenceSliceReader, getStats, getGapRegions, etc.), whose signatures are unchanged here.
  • Ran gff3tools' full test suite (1128 tests) against this branch's build with zero source changes on the gff3tools side — only the dependency version bumped. Same pass/fail result as against the current release (2 pre-existing, environment-only failures unrelated to this change).

So for consumers, adopting this once released will be a plain dependency version bump.

Minor note (not blocking)

./gradlew spotlessCheck currently reports a couple of trivial formatting violations on this branch (SequenceIndexBuilder.java trailing blank line, LineIndexTest.java line wrapping) — ./gradlew spotlessApply before merge should clear them.

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.

1 participant