airlift / aircompressor

A port of Snappy, LZO, LZ4, and Zstandard to Java
Apache License 2.0
562 stars 111 forks source link

Split Zstd decompression out from frame and header handling. #149

Closed arnej27959 closed 1 year ago

arnej27959 commented 2 years ago

This PR is almost all moving code into a new file; in order to do frame handling separate from the actual block decompression. This enables (in a later PR) a separate streaming decompressor which needs its own frame and buffer handling.

arnej27959 commented 2 years ago

@martint or @findepi please review

bondolo commented 2 years ago

I am interested in the same functionality for implementing zstd decompression support for Netty. I would like to be able to decompress individual blocks as they arrive via HTTP streaming. The netty buffers aren't necessarily aligned to block boundaries. Initially, at least, it will be OK to only process complete blocks but perhaps later incremental processing of blocks would be added.

dain commented 2 years ago

My guess looking at this patch is that is has a large negative impact on performance for anyone processing multiple frames because it does not reuse buffers across frame decoding. PR #152 that I recently put up that adds ZstdInputStream and ZstdOutputStream, and I added a partialDecompress that might be able to do what you want here, although I'm not sure the internals will be exposed directly. It would be more helpful to create an issue with the specific use case and proposed interface you are looking for.

arnej27959 commented 2 years ago

My guess looking at this patch is that is has a large negative impact on performance for anyone processing multiple frames because it does not reuse buffers across frame decoding.

there's not even a tiny bit of buffer handling here - it just takes the buffers as input arguments, just like before.

Indeed, looking at PR #152 it would be beneficial for that PR too to do this split first, as the API that's moved to ZstdBlockDecompressor here is exactly what you need in your partialDecompress method.

I have also implemented a version of ZstdInputStream but I guess that's moot now; but I'll make a PR anyway so you can compare the buffer handling / layering if you want to.

dain commented 1 year ago

Zstd streaming was implemented in #152