Skip to content

perf(concat): block-copy concatenation instead of SeqRecord round-trip (89x faster, -2.6 GB) - #5

Open
sanjaynagi-eit wants to merge 1 commit into
mainfrom
perf/concat-and-fasta-io
Open

perf(concat): block-copy concatenation instead of SeqRecord round-trip (89x faster, -2.6 GB)#5
sanjaynagi-eit wants to merge 1 commit into
mainfrom
perf/concat-and-fasta-io

Conversation

@sanjaynagi-eit

Copy link
Copy Markdown
Owner

Problem

concatenate_single_fastq parses both input FASTQs into a python list of BioPython SeqRecords and then writes them out again:

records = []
records.extend(SeqIO.parse(handle, "fastq"))   # x2
SeqIO.write(records, handle, "fastq")

SeqRecord overhead is roughly 5-10x the file size, so concatenating the short reads of a hybrid run can transiently need tens of GB. concatenate_single_fasta does the same for contigs, and Plass.get_depth_long / Assembly.combine_input_fastas each carry their own copy of the list(SeqIO.parse(...)) habit.

Concatenating records requires no parsing at all.

Change

Copy the inputs in 1 MiB blocks, decompressing gzipped sources on the way through. Two details worth calling out:

  • Newline safety: a source whose final line is unterminated would otherwise run straight into the next file's first header. _append_file appends a newline when the copied data does not end with one.
  • Format validation: parsing every record used to catch a wrong-format input, and test_concat_single_fastq_bad depends on that. The first byte is now checked against the format's record marker (@ / >) — same guarantee, O(1) instead of O(file).

Plass.get_depth_long now calls concatenate_single_fasta instead of building a list of records. Assembly.combine_input_fastas genuinely has to parse (it renames contigs) but streams them rather than materialising the assembly as a list.

Measurements

573 MiB of ONT FASTQ, each implementation in a fresh process:

before after
time 28.44 s 0.32 s 89x
peak RSS 2672 MB 88 MB -2.6 GB

Output is byte-identical, and the parsed (id, sequence, qualities) records match exactly.

One thing deliberately left alone

Assembly.combine_input_fastas contains:

for record in records:
    ...
    record.id = str(i)
    records[0].description = ""   # <- always index 0, inside the loop

That clears only the first plasmid's description while every later one keeps its own — almost certainly a copy-paste slip. It matters, because get_contig_circularity() decides circularity by looking for "circular" in the description. Fixing it would change results, so the behaviour is preserved exactly and flagged with a comment for a separate PR.

Tests

New tests/test_concat.py: record preservation for FASTQ and FASTA, gzipped input, empty input, the missing-trailing-newline case, wrong-format rejection for both directions, and that FASTA descriptions survive (circularity depends on them). All existing tests pass (76 non-slow).

…ords

concatenate_single_fastq parsed both inputs into a list of BioPython
SeqRecords and rewrote them. SeqRecord overhead is roughly 5-10x the file
size, so concatenating short reads in a hybrid run could transiently need
tens of GB. concatenate_single_fasta did the same for contigs, and
Plass.get_depth_long / Assembly.combine_input_fastas had their own copies of
the list(SeqIO.parse(...)) habit.

Concatenating records requires no parsing. Copy in 1 MiB blocks instead,
decompressing gzipped inputs on the way through, and guarantee a newline
between files so a source with no trailing newline cannot run its last line
into the next file's header.

Measured on 573 MiB of ONT fastq:
  seconds       28.44 -> 0.32     (89x)
  peak RSS    2672 MB -> 88 MB    (-2.6 GB)
with byte-identical output and identical (id, sequence, qualities) records.

Parsing every record used to catch a wrong-format input, and a test relies on
that, so _append_file checks the first byte instead - O(1) rather than
O(file). Empty inputs are still fine; they are normal when there are no
unmapped reads.

Assembly.combine_input_fastas genuinely has to parse, because it renames
contigs, but it now streams records rather than materialising the whole
assembly as a list first. Its odd 'records[0].description = ""' inside the
per-record loop - which only ever cleared the first plasmid's description,
while later ones kept theirs - is preserved with a comment, because
get_contig_circularity() reads that description and changing it would change
results.
@sanjaynagi-eit
sanjaynagi-eit force-pushed the perf/concat-and-fasta-io branch from 4ec43d1 to 1147f80 Compare August 13, 2026 21:46
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