airlift / aircompressor

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

Make ZstdFrameCompressor public #101

Closed merlimat closed 1 year ago

merlimat commented 5 years ago

In order to do direct compression on a memory region, it would be good to be able to call ZstdFrameCompressor.compress() without intermediaries.

martint commented 5 years ago

Can you elaborate on what you're trying to achieve? The compressor already has two public methods for compressing in-heap or off-heap data:

https://github.com/airlift/aircompressor/blob/master/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java#L40 https://github.com/airlift/aircompressor/blob/master/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java#L49

merlimat commented 5 years ago

I'm using Netty's ByteBuf to pool memory buffers. Typically, these will be backed up by direct memory and I can get access to the underlying memoryAddress.

While a Netty ByteBuf can also be converted into a JDK ByteBuffer (pointing to the same memory), that would typically involve allocating the new ByteBuffer object. Compressing directly from and to the memory region will avoid these allocations.

Also, this is already the behavior exposed for the other compressors (lz4, snappy have already the "raw" compressor as a public class).

martint commented 5 years ago

In that case, it'd be better to add a dedicated method to ZstdCompressor that takes a raw address+length. It should be clearly document as inherently unsafe.

Also, this is already the behavior exposed for the other compressors (lz4, snappy have already the "raw" compressor as a public class).

That's an oversight. Those classes should be private.