saves 3-bytes on small input with streaming API

zstd streaming API was adding a null-block at end of frame for small input.

Reason is : on small input, a single block is enough.
ZSTD_CStream would size its input buffer to expect a single block of this size,
automatically triggering a flush on reaching this size.

Unfortunately, that last byte was generally received before the "end" directive (at least in `fileio`).
The later "end" directive would force the creation of a 3-bytes last block to indicate end of frame.

The solution is to not flush automatically, which is btw the expected behavior.
It happens in this case because blocksize is defined with exactly the same size as input.
Just adding one-byte is enough to stop triggering the automatic flush.

I initially looked at another solution, solving the problem directly in the compression context.
But it felt awkward.
Now, the underlying compression API `ZSTD_compressContinue()` would take the decision the close a frame
on reaching its expected end (`pledgedSrcSize`).
This feels awkward, a responsability over-reach, beyond the definition of this API.
ZSTD_compressContinue() is clearly documented as a guaranteed flush,
with ZSTD_compressEnd() generating a guaranteed end.

I faced similar issue when trying to port a similar mechanism at the higher streaming layer.
Having ZSTD_CStream end a frame automatically on reaching `pledgedSrcSize` can surprise the caller,
since it did not explicitly requested an end of frame.
The only sensible action remaining after that is to end the frame with no additional input.
This adds additional logic in the ZSTD_CStream state to check this condition.
Plus some potential confusion on the meaning of ZSTD_endStream() with no additional input (ending confirmation ? new 0-size frame ?)

In the end, just enlarging input buffer by 1 byte feels the least intrusive change.
It's also a contract remaining inside the streaming layer, so the logic is contained in this part of the code.

The patch also introduces a new test checking that size of small frame is as expected, without additional 3-bytes null block.

diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c
index 0dec78820b21c9c5622ddfdfdea0e3f08a403ffe..37dd5a4b0b1ac1f5a9f6fd17990a312d016c0667 100644 (file)
--- a/lib/compress/zstd_compress.c
+++ b/lib/compress/zstd_compress.c
@@ -2527,7 +2527,8 @@ static size_t ZSTD_resetCStream_internal(ZSTD_CStream* zcs,
 
     zcs->inToCompress = 0;
     zcs->inBuffPos = 0;
-    zcs->inBuffTarget = zcs->blockSize;
+    zcs->inBuffTarget = zcs->blockSize
+                      + (zcs->blockSize == pledgedSrcSize);   /* for small input: avoid automatic flush on reaching end of block, since it would require to add a 3-bytes null block to end frame */
     zcs->outBuffContentSize = zcs->outBuffFlushedSize = 0;
     zcs->streamStage = zcss_load;
     zcs->frameEnded = 0;
@@ -2679,9 +2680,9 @@ size_t ZSTD_compressStream_generic(ZSTD_CStream* zcs,
     /* check expectations */
     DEBUGLOG(5, "ZSTD_compressStream_generic, flush=%u", (U32)flushMode);
     assert(zcs->inBuff != NULL);
-    assert(zcs->inBuffSize>0);
-    assert(zcs->outBuff!= NULL);
-    assert(zcs->outBuffSize>0);
+    assert(zcs->inBuffSize > 0);
+    assert(zcs->outBuff !=  NULL);
+    assert(zcs->outBuffSize > 0);
     assert(output->pos <= output->size);
     assert(input->pos <= input->size);
 

diff --git a/tests/playTests.sh b/tests/playTests.sh
index 9db3dadea3b5dc467f9e42176ef868a857d2c622..c48148fa295e84d2834fe14c9db6a514f67f6d7f 100755 (executable)
--- a/tests/playTests.sh
+++ b/tests/playTests.sh
@@ -126,6 +126,8 @@ $ZSTD -d > $INTOVOID && die "should have refused : compressed data from terminal
 fi
 $ECHO "test : null-length file roundtrip"
 $ECHO -n '' | $ZSTD - --stdout | $ZSTD -d --stdout
+$ECHO "test : ensure small file doesn't add 3-bytes null block"
+./datagen -g1 | $ZSTD | wc -c | grep "14"
 $ECHO "test : decompress file with wrong suffix (must fail)"
 $ZSTD -d tmpCompressed && die "wrong suffix error not detected!"
 $ZSTD -df tmp && die "should have refused : wrong extension"
@@ -324,7 +326,7 @@ $DIFF $TESTFILE result
 if [ -n "$hasMT" ]
 then
     $ECHO "- Test dictionary compression with multithreading "
-    ./datagen -g5M | $ZSTD -T2 -D tmpDict | $ZSTD -t -D tmpDict   # fails with v1.3.2 
+    ./datagen -g5M | $ZSTD -T2 -D tmpDict | $ZSTD -t -D tmpDict   # fails with v1.3.2
 fi
 $ECHO "- Create second (different) dictionary "
 $ZSTD --train *.c ../programs/*.c ../programs/*.h -o tmpDictC