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

Enable IntelInflater #1885

Merged
merged 2 commits into from
Jun 2, 2023
Merged

Enable IntelInflater #1885

merged 2 commits into from
Jun 2, 2023

Conversation

lbergelson
Copy link
Member

Fixes a long standing bug where we were accidentally not using the intel inflator when decompressing even when it was requested. Includes a horrible test using brittle reflection.

*  We have been using the Java Inflater even when the Intel one was available and requested
   due to a bug in the configuration of the default inflater.
   The fix is fairly brittle because it depends on the order of initialization of various things.
*  This will be fixed more generally by samtools/htsjdk#1666
Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

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

This is fine for now as a quick fix 👍 However, we should definitely patch HTSJDK to improve safety/consistency around updating the cached defaults so we don't run into this again at some point.

@droazen
Copy link
Contributor

droazen commented Jun 2, 2023

@lbergelson Can you confirm that the test you added fails without your patch?

@lbergelson
Copy link
Member Author

@droazen Yes, I tested without my patch and the inflater test fails. The deflater one passes because it was working before my patch.

@lbergelson lbergelson merged commit 983e073 into master Jun 2, 2023
@lbergelson lbergelson deleted the lb_fix_decompression branch June 2, 2023 19:50
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