Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: speedup fasta reader #296

Merged
merged 1 commit into from
Dec 10, 2024
Merged

perf: speedup fasta reader #296

merged 1 commit into from
Dec 10, 2024

Conversation

ivan-aksamentov
Copy link
Member

Here I restructured FastaReader::read() function to avoid extra allocations. This is basically a complete rewrite.

This involved changing error handling, so I thought while we are at it, why not improve error messages.

While working on the new algo, I also discovered a few important cases missing from tests, so I added them.

Speedup on mpox-500 run: ~29%

Command:

$ cargo -q build --release --target-dir=/workdir/.build/docker --bin=treetime
$ hyperfine --warmup 1 --show-output '/workdir/.build/docker/release/treetime ancestral --method-anc=parsimony --dense=false --tree=data/mpox/clade-ii/500/tree.nwk --outdir=tmp/smoke-tests/ancestral/marginal/mpox/clade-ii/500 data/mpox/clade-ii/500/aln.fasta.xz'

Before (branch: rust, commit f676392):

  Time (mean ± σ):      1.866 s ±  0.113 s    [User: 4.547 s, System: 0.576 s]
  Range (min … max):    1.768 s …  2.041 s    10 runs

After:

  Time (mean ± σ):      1.444 s ±  0.111 s    [User: 4.181 s, System: 0.527 s]
  Range (min … max):    1.341 s …  1.650 s    10 runs

Profiler shows that treetime::io::fasta::FastaReader::read() now takes ~18% of the program, down from 25% (in single-threaded mode).

Here I restructured `FastaReader::read()` function to avoid extra allocations. This is basically a complete rewrite.

This involved changing error handling, so I thought while we are at it, why not improve error messages.

While working on the new algo, I also discovered a few important cases missing from tests, so I added them.

Speedup on mpox-500 run: ~29%

Command:
```
$ cargo -q build --release --target-dir=/workdir/.build/docker --bin=treetime
$ hyperfine --warmup 1 --show-output '/workdir/.build/docker/release/treetime ancestral --method-anc=parsimony --dense=false --tree=data/mpox/clade-ii/500/tree.nwk --outdir=tmp/smoke-tests/ancestral/marginal/mpox/clade-ii/500 data/mpox/clade-ii/500/aln.fasta.xz'
```

Before (branch: rust, commit f676392):
```
  Time (mean ± σ):      1.866 s ±  0.113 s    [User: 4.547 s, System: 0.576 s]
  Range (min … max):    1.768 s …  2.041 s    10 runs
```

After:
```
  Time (mean ± σ):      1.444 s ±  0.111 s    [User: 4.181 s, System: 0.527 s]
  Range (min … max):    1.341 s …  1.650 s    10 runs
```

Profiler shows that `treetime::io::fasta::FastaReader::read()` now takes ~18% of the program, down from 25% (in single-threaded mode).
@ivan-aksamentov ivan-aksamentov merged commit 66880d7 into rust Dec 10, 2024
14 checks passed
@ivan-aksamentov ivan-aksamentov deleted the perf/fasta-reader branch December 10, 2024 19:32
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