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

[HLS-627] Improve lift_over_variants performance #285

Merged
merged 1 commit into from
Aug 26, 2020

Conversation

kianfar77
Copy link
Collaborator

Signed-off-by: kianfar77 [email protected]

What changes are proposed in this pull request?

This PR removed the usage of htsjdk.samtools.reference.ReferenceSequence.getBaseString on long sequences and replaces it with getBases followed by converting only the desired part of the byte array to string.

Before this change, on a small cluster the transformer took close to 2 hours to do liftover on 1000 genome chr22 vcf due to a lot of string allocation and GC. After this change, the liftover takes about 10min.

How is this patch tested?

  • Unit tests
  • Integration tests
  • Manual tests

(Details)

Signed-off-by: kianfar77 <[email protected]>
@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

Merging #285 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #285   +/-   ##
=======================================
  Coverage   93.62%   93.63%           
=======================================
  Files          92       92           
  Lines        4364     4367    +3     
  Branches      386      380    -6     
=======================================
+ Hits         4086     4089    +3     
  Misses        278      278           
Impacted Files Coverage Δ
...low/transformers/LiftOverVariantsTransformer.scala 99.42% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e344369...bca160e. Read the comment docs.

Copy link
Contributor

@henrydavidge henrydavidge left a comment

Choose a reason for hiding this comment

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

LGTM

@kianfar77 kianfar77 merged commit 914bcd3 into projectglow:master Aug 26, 2020
kianfar77 added a commit to kianfar77/glow that referenced this pull request Sep 4, 2020
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