Integrate jlibdeflate for faster DEFLATE compression and decompression#1768
Merged
Integrate jlibdeflate for faster DEFLATE compression and decompression#1768
Conversation
nh13
requested changes
Apr 5, 2026
Member
nh13
left a comment
nh13
left a comment
There was a problem hiding this comment.
Looking good, just a few questions and one extra C
|
|
||
| task.jvmArgs '-Djava.awt.headless=true' //this prevents awt from displaying a java icon while the tests are running | ||
|
|
||
| C |
| } | ||
|
|
||
| dependencies { | ||
| implementation 'com.fulcrumgenomics:jlibdeflate:0.1.0-SNAPSHOT' |
| @@ -0,0 +1,121 @@ | |||
| package htsjdk.samtools.util.zip; | |||
Member
There was a problem hiding this comment.
nitpick: all other files have licenses in their header, should we follow that same convention
| } | ||
|
|
||
| /** Returns true if the libdeflate native library can be loaded. */ | ||
| static boolean isLibdeflateAvailable() { |
Member
There was a problem hiding this comment.
nitpick: it seems odd that InflaterFactory calls a method on DeflatorFactory, but putting it into it's own separate class seems odd too. 🤷
Member
Author
There was a problem hiding this comment.
Agreed. Feels like it should just be CompressionFactory, but consolidating the two would be a bigger breaking change now.
tfenne
force-pushed
the
tf_bam_optimization
branch
from
3fa0f9f to
4d694af
Compare
Add optional jlibdeflate (libdeflate JNI bindings) support that is automatically used when the native library is available on the classpath, with transparent fallback to the JDK Inflater/Deflater. - DeflaterFactory/InflaterFactory auto-detect jlibdeflate availability at first use, caching the result for the lifetime of the JVM - LibdeflateDeflater/LibdeflateInflater extend JDK Deflater/Inflater and correctly handle both raw DEFLATE (nowrap=true) and zlib format (nowrap=false) - New Defaults.USE_LIBDEFLATE (samjdk.use_libdeflate) defaults to true; set to false to force JDK-only compression - jlibdeflate 0.1.0-SNAPSHOT added as a dependency from Sonatype Central snapshots
Co-authored-by: Nils Homer <nh13@users.noreply.github.com>
tfenne
force-pushed
the
tf_jlibdeflate_integration
branch
from
d21cc0b to
abfe4a6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add optional jlibdeflate (libdeflate JNI bindings) support that is automatically used when the native library is available on the classpath, with transparent fallback to the JDK Inflater/Deflater.
It should be noted that
jlibdeflate(https://github.com/fulcrumgenomics/jlibdeflate) is brand new, and is developed specifically to be used here. On pure BAM roundtrip on mac/aarch64 it is more than 2x faster than the built in jdk/zlib implementation!! Once we reach consensus on this PR I'll create a first release of jlibdeflate and publish to sonatype, and amend this PR.Things to think about before submitting: