Implement GzipByteBuffDecompressor with on-heap and off-heap decompression paths - #8541
Implement GzipByteBuffDecompressor with on-heap and off-heap decompression paths#8541SaadASTheDev wants to merge 1 commit into
Conversation
| private static final int GZIP_HEADER_LENGTH = 10; | ||
| private static final int GZIP_TRAILER_LENGTH = 8; | ||
|
|
||
| @Nullable |
There was a problem hiding this comment.
These are the Singleton objects reused across decompress calls, Allocated once per buffer, to avoid GC overhead
| allowByteBuffDecompression = true; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
The flow for wether we can decompress is
1: verify config via context to ensure we havent disabled this feature before we try to decompress via bytebuff
2. we only support singleByteBuff decompression for now
3. Since Zlib is a JNI we need to ensure that its properly instantiated if and only if the buffers we are trying to decompress are direct, so if both are direct we ensure the decompressor is loaded
4. then we return if both I/O are not direct, if true it uses the inflater pathway
| } | ||
| return decompressOnHeap(nioInput, nioOutput, inputLen); | ||
| } | ||
|
|
There was a problem hiding this comment.
Respective off heap decompress which uses the zlib library and verifies the trailer on its own
| } | ||
|
|
||
| @Override | ||
| public void reinit(@Nullable Compression.HFileDecompressionContext newHFileDecompressionContext) { |
There was a problem hiding this comment.
We load in a new context at every new Hfile, it would be too expensive to do it at every block
628789b to
cedb75b
Compare
| @Nullable | ||
| private final ZlibDecompressor.ZlibDirectDecompressor decompressor; | ||
|
|
||
| private final Inflater inflater = new Inflater(true); |
There was a problem hiding this comment.
For consistency, I suggest you use the top-level ZlibDecompressor to handle on-heap
There was a problem hiding this comment.
Will do, wasn't familiar with the top level decompressor but that simplifies things, Thanks!
| while (!decompressor.finished()) { | ||
| int outputRemainingBefore = nioOutput.remaining(); | ||
| try { | ||
| decompressor.decompress(gzipMember, nioOutput); | ||
| } catch (IOException e) { | ||
| throw new IOException("Invalid gzip stream: " + e.getMessage(), e); | ||
| } | ||
| if (nioOutput.remaining() == outputRemainingBefore && !decompressor.finished()) { | ||
| if (!nioOutput.hasRemaining()) { | ||
| throw new IOException("Output buffer is too small for the decompressed gzip stream"); | ||
| } | ||
| throw new IOException("Unexpected end of gzip stream"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason this needs to loop? Why would the decompressor need multiple attempts?
There was a problem hiding this comment.
Yea It doesnt need to loop. I was under the impression the zlib decompressor only follows a streaming, chunk-based pattern, where a block gets decompressed in chunks. Thats only true if we don't pre size the output buffer, going to fix this, Thanks
| /* | ||
| * "HBase is fun to use and very fast" compressed as a single gzip member via GZIPOutputStream, | ||
| * matching the framing that ReusableStreamGzipCodec produces on the compression side. | ||
| */ |
There was a problem hiding this comment.
Are there are situations where a gzip input payload would contain multiple members?
There was a problem hiding this comment.
There are situations, the payload will contain multiple members but we enclose over ours via: InputStart + InputLen, but we should test to make sure we did it correctly
…for efficient GZIP decompression
cedb75b to
1407839
Compare
2 participants
Why
HBase's
ByteBuffDecompressorinterface enables block decompression directly from/toByteBufferobjects, avoiding intermediate byte array copies. GZIP had no implementationof this interface — all GZIP decompression went through a byte array round-trip regardless
of whether the underlying buffers were on-heap or off-heap.
What
Adds
GzipByteBuffDecompressor, aByteBuffDecompressorfor GZIP that handles two cases:ZlibDirectDecompressorwith
GZIP_FORMAT. Requires native zlib to be loaded;canDecompress()returns falseif it isn't.
Inflaterin raw DEFLATE (nowrap) mode,skipping the 10-byte GZIP header manually and verifying the CRC32 and ISIZE trailer
fields after inflation. The native
ZlibDirectDecompressorrequires a stable nativememory address, which heap
ByteBuffers do not provide, making this fallback necessary.Also adds:
GzipHFileDecompressionContext— carries per-context configuration (e.g. whetherByteBuff decompression is allowed), wired in via
reinit()(header + trailer = 18 bytes)
Testing
canDecompress()guard logic