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

Misc performance improvements for CRAM reader #304

Merged
merged 4 commits into from
Apr 8, 2024

Conversation

athos
Copy link
Member

@athos athos commented Apr 3, 2024

This PR tweaks the performance of the CRAM reader by the following changes:

  • 16c4a00: Use lookup tables to look up rANS symbols, instead of doing binary search
  • 2221b5e: Replace maps and vectors used in the rANS decoder with arrays
  • 2dc6ae7: Coerce decoded quals to make = more efficient
  • 154426f: Replace + with bit-or

Overall, these changes make the CRAM reader roughly 4x faster than before on my environment:

(with-open [r (reader "DRR002191.chr1.sorted.cram" {:reference ".cavia/hg38.2bit"})]
  (time (count (take 1000000 (read-alignments r)))))

;; before change
"Elapsed time: 22955.078416 msecs"

;; after change
"Elapsed time: 5079.17675 msecs"

Here are the profiling results before and after the change:

before change after change
flamegraph before change flamegraph after change

@athos athos self-assigned this Apr 3, 2024
@athos athos requested review from alumi and a team as code owners April 3, 2024 05:06
@athos athos requested review from r6eve and removed request for a team April 3, 2024 05:06
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.67%. Comparing base (43df52d) to head (154426f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #304   +/-   ##
=======================================
  Coverage   88.67%   88.67%           
=======================================
  Files          93       93           
  Lines        8098     8100    +2     
  Branches      505      505           
=======================================
+ Hits         7181     7183    +2     
  Misses        412      412           
  Partials      505      505           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@athos athos assigned alumi and r6eve Apr 3, 2024
Copy link
Member

@alumi alumi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the Pull Request!
With the speed improvements you implemented, I believe it's becoming more practical to use. Great job! 👍

Copy link
Contributor

@r6eve r6eve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for the PR.

@r6eve r6eve merged commit 4699b7f into master Apr 8, 2024
18 checks passed
@r6eve r6eve deleted the feature/cram-decoder-perf branch April 8, 2024 07:00
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.

3 participants