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

fix: use array instead of Vec for Codon representation (and other free perf improvements) #169

Merged
merged 1 commit into from
May 8, 2024

Conversation

tedil
Copy link
Contributor

@tedil tedil commented Apr 30, 2024

  • use [u8; 3] instead of Vec, i.e. stack vs heap
  • use ahash instead of fxhash
  • use array slicing &c[..3] instead of iter().take(3) (even though that should not really have made a difference, but according to criterion, seemed to be the case)
  • make translate a little more readable with or_else chaining

Copy link

codecov bot commented Apr 30, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 87.42%. Comparing base (721d606) to head (2b08951).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
- Coverage   87.42%   87.42%   -0.01%     
==========================================
  Files          18       18              
  Lines        7700     7705       +5     
==========================================
+ Hits         6732     6736       +4     
- Misses        968      969       +1     
Files Coverage Δ
src/sequences.rs 98.28% <83.33%> (-0.11%) ⬇️

@xiamaz
Copy link

xiamaz commented Apr 30, 2024

Thanks a lot. This generally looks good. How much performance impact are you seeing in a test case?

@tedil tedil force-pushed the perf-codon-repr branch from 6a10a6d to 2b08951 Compare April 30, 2024 15:14
@tedil
Copy link
Contributor Author

tedil commented Apr 30, 2024

main branch

     Running benches/translate_cds.rs (target/release/deps/translate_cds-c57b7f59d5053bc8)
translate_cds TTN       time:   [153.05 µs 153.31 µs 153.61 µs]
Found 14 outliers among 100 measurements (14.00%)
  5 (5.00%) high mild
  9 (9.00%) high severe

perf-codon-repr branch

     Running benches/translate_cds.rs (target/release/deps/translate_cds-41f293dcbeaaadcb)
translate_cds TTN       time:   [120.43 µs 121.08 µs 121.87 µs]
                        change: [-20.521% -19.639% -18.312%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  3 (3.00%) high mild
  8 (8.00%) high severe

@tedil tedil merged commit e455168 into main May 8, 2024
7 of 8 checks passed
@tedil tedil deleted the perf-codon-repr branch May 8, 2024 13:13
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