airlift / aircompressor

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

ZstdCompressor.maxCompressedLength is unnecessarily pessimistic #113

Closed danburkert closed 4 years ago

danburkert commented 4 years ago

ZstdCompressor.maxCompressedLength isn't an accurate translation of the ZSTD_COMPRESSBOUND macro due to some missing & misplaced parenthesis. I believe the following diff would be more accurate. Not sending a PR because I haven't looked into tests.

diff --git a/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java b/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java
index 23ace52..b714d63 100644
--- a/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java
+++ b/src/main/java/io/airlift/compress/zstd/ZstdCompressor.java
@@ -30,7 +30,7 @@ public class ZstdCompressor
         int result = uncompressedSize + (uncompressedSize >>> 8);

         if (uncompressedSize < MAX_BLOCK_SIZE) {
-            result += MAX_BLOCK_SIZE - (uncompressedSize >>> 11);
+            result += ((MAX_BLOCK_SIZE - uncompressedSize) >>> 11);
         }

         return result;
martint commented 4 years ago

Good catch! Thanks for finding and reporting this. I've put up a PR to fix it: #114