airlift / aircompressor

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

Make SnappyCompressor stateless #121

Closed Shadi closed 3 years ago

Shadi commented 3 years ago

moved the table field to be a local variable instead of an object instance, this should make this service stateless and avoid synchronization or creating new instances in multi-threaded applications.

dain commented 3 years ago

The compressors are designed this way so that the table allocation can be reused. When compressing small things, the allocation can dominate the performance of the compression. You can achieve this exact same design by simply creating a new SnappyCompressor each time you want to compress. It is likely that the JVM will just inline this code into the call site so you won't even get an allocation of the SnappyCompressor object.

Shadi commented 3 years ago

Hi Dain, I'm already creating new instances of SnappyCompressor, at first I wanted to create this PR to add a comment to SnappyCompressor to indicate its not thread-safe(same as here https://github.com/airlift/aircompressor/blob/master/src/main/java/io/airlift/compress/lz4/Lz4Compressor.java#L25) but when I looked at the code I saw that it can be made thread-safe by moving this field, if this will have adverse effect on the compression performance then feel free to close it.

dain commented 3 years ago

Thanks for looking into this!