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

Bring in some performance improvements from genomicsdb #9059

Merged
merged 3 commits into from
Feb 26, 2025

Conversation

nalinigans
Copy link
Collaborator

No description provided.

@nalinigans
Copy link
Collaborator Author

@gokalpcelik can you check if the improvements in branch [genomicsdb_perf](https://github.com/broadinstitute/gatk/tree/genomicsdb_perf) help with #8989?

@lbergelson
Copy link
Member

@nalinigans Thank you!

@nalinigans
Copy link
Collaborator Author

nalinigans commented Jan 6, 2025

@gokalpcelik, any updates on whether you could test the performance improvements? If yes, I can make a maven product release of GenomicsDB. Thanks.

@gokalpcelik
Copy link
Contributor

@nalinigans Indeed yes. I was able to confirm that memory usage is much better now although not at 4.1 levels but still it does not die on OOM. Fidelity is also OK in my testing so if the team says OK and you are OK we can give it a go.

@gokalpcelik
Copy link
Contributor

gokalpcelik commented Jan 6, 2025

@nalinigans
I remember that you said it is slower now compared to older implementation. My findings are also similar. It is slower (about 1.1~1.3 times) compared to 4.1 and 4.2 and beyond. Did you have a chance to add additional tweaks that you mentioned?
No, I have not a chance for additional tweaks yet. But, will keep it in mind to check this soon.

@nalinigans
Copy link
Collaborator Author

nalinigans commented Jan 31, 2025

@gokalpcelik @lbergelson, I have changed the snapshot release used for testing to the released genomicsdb 1.5.5 jar. Please feel free to review and pull in this PR. Thanks.

Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

Testing shows this is a huge improvement memory wise.

@lbergelson lbergelson merged commit 80f8da3 into master Feb 26, 2025
20 checks passed
@lbergelson lbergelson deleted the genomicsdb_perf branch February 26, 2025 17:34
@lbergelson
Copy link
Member

Thank you @nalinigans and @gokalpcelik

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