diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b67fd7cc..589bda54 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,13 @@ Changelog version 1.10.0-dev ------------------ ++ Make ``isal_zlib.Compress.flush()`` return ``b""`` for ``Z_SYNC_FLUSH`` + when nothing was compressed since a sync or full flush, and for + ``Z_FULL_FLUSH`` when nothing was compressed since a full flush, as zlib + does. It wrote an empty block of 6 bytes, or 117 bytes at level 0. On + Linux, such a flush is now 20 times faster at levels 1 to 3. Flushing an + ``igzip`` file again without a write in between no longer grows the file + and is 4 times faster. + Fix ``python -m isal.igzip`` replacing the input file with empty output and exiting with status 0 when the output file is the input file, as in ``-f -o data data``. It exits with an error now. diff --git a/src/isal/isal_zlibmodule.c b/src/isal/isal_zlibmodule.c index 3ee12d83..b6eecfbf 100644 --- a/src/isal/isal_zlibmodule.c +++ b/src/isal/isal_zlibmodule.c @@ -584,6 +584,8 @@ typedef struct PyObject *zdict; struct isal_zlib_header zlib_header; int is_initialised; + /* Mode of the last flush; Z_NO_FLUSH after compress() or Z_FINISH. */ + int last_flush; PyThread_type_lock lock; // isal_zstream should be at the bottom as it contains buffers inside the struct. struct isal_zstream zst; @@ -609,6 +611,7 @@ newcompobject() if (self == NULL) return NULL; self->is_initialised = 0; + self->last_flush = Z_NO_FLUSH; self->zdict = NULL; self->zlib_header.dict_flag = 0; self->level_buf = NULL; @@ -898,6 +901,8 @@ isal_zlib_Compress_compress_impl(compobject *self, Py_buffer *data) ENTER_ZLIB(self); + /* Also for b"", as in zlib: permessage-deflate needs the sync marker. */ + self->last_flush = Z_NO_FLUSH; self->zst.next_in = data->buf; ibuflen = data->len; @@ -1117,6 +1122,13 @@ isal_zlib_Compress_flush_impl(compobject *self, int mode) ENTER_ZLIB(self); + /* As in zlib, skip the empty block after an equal or stronger flush. */ + if ((mode == Z_SYNC_FLUSH || mode == Z_FULL_FLUSH) && + self->last_flush >= mode) { + LEAVE_ZLIB(self); + return PyBytes_FromStringAndSize(NULL, 0); + } + if (mode == Z_FINISH) { self->zst.flush = FULL_FLUSH; self->zst.end_of_stream = 1; @@ -1167,9 +1179,11 @@ isal_zlib_Compress_flush_impl(compobject *self, int mode) self->zst.level_buf_size = 0; self->zst.level_buf = NULL; self->is_initialised = 0; + self->last_flush = Z_NO_FLUSH; } else { // reset the flush mode back so compressobject can be used again. self->zst.flush = NO_FLUSH; + self->last_flush = mode; } if (_PyBytes_Resize(&RetVal, self->zst.next_out - diff --git a/tests/test_compat.py b/tests/test_compat.py index 3f339851..4db3765c 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -174,6 +174,63 @@ def test_compressobj_level_0_flush_after_finish(): assert zlib.decompress(compressed) == b"data" +@pytest.mark.parametrize("level", range(4)) +@pytest.mark.parametrize("wbits", [-15, 15, 31]) +@pytest.mark.parametrize(["first_mode", "second_mode"], + [(isal_zlib.Z_SYNC_FLUSH, isal_zlib.Z_SYNC_FLUSH), + (isal_zlib.Z_FULL_FLUSH, isal_zlib.Z_FULL_FLUSH), + (isal_zlib.Z_FULL_FLUSH, isal_zlib.Z_SYNC_FLUSH)]) +def test_compressobj_repeated_flush(level, wbits, first_mode, second_mode): + compressor = isal_zlib.compressobj(level, wbits=wbits) + decompressor = zlib.decompressobj(wbits=wbits) + compressed = (compressor.compress(DATA[:4096]) + + compressor.flush(first_mode)) + assert compressed.endswith(b"\x00\x00\xff\xff") + assert decompressor.decompress(compressed) == DATA[:4096] + assert compressor.flush(second_mode) == b"" + assert compressor.flush(second_mode) == b"" + compressed = compressor.compress(DATA[4096:8192]) + compressor.flush() + assert decompressor.decompress(compressed) == DATA[4096:8192] + assert decompressor.eof + + +@pytest.mark.parametrize("level", range(4)) +@pytest.mark.parametrize("wbits", [-15, 15, 31]) +def test_compressobj_repeated_flush_before_data(level, wbits): + compressor = isal_zlib.compressobj(level, wbits=wbits) + compressed = compressor.flush(isal_zlib.Z_SYNC_FLUSH) + assert compressed.endswith(b"\x00\x00\xff\xff") + assert compressor.flush(isal_zlib.Z_SYNC_FLUSH) == b"" + compressed += compressor.compress(DATA[:4096]) + compressor.flush() + assert zlib.decompress(compressed, wbits=wbits) == DATA[:4096] + + +@pytest.mark.parametrize("level", range(4)) +def test_compressobj_full_flush_after_sync_flush(level): + compressor = isal_zlib.compressobj(level, wbits=-15) + compressor.compress(DATA[:4096]) + compressor.flush(isal_zlib.Z_SYNC_FLUSH) + # A full flush after a sync flush still resets the history. + assert compressor.flush(isal_zlib.Z_FULL_FLUSH).endswith( + b"\x00\x00\xff\xff") + compressed = compressor.compress(DATA[:4096]) + compressor.flush() + assert zlib.decompress(compressed, wbits=-15) == DATA[:4096] + + +@pytest.mark.parametrize("level", range(4)) +@pytest.mark.parametrize("flush_mode", [isal_zlib.Z_SYNC_FLUSH, + isal_zlib.Z_FULL_FLUSH]) +def test_compressobj_flush_after_empty_compress(level, flush_mode): + # permessage-deflate (RFC 7692) needs 00 00 ff ff after empty messages. + compressor = isal_zlib.compressobj(level, wbits=-15) + decompressor = zlib.decompressobj(wbits=-15) + for message in [b"hello", b"", b"", DATA[:4096], b""]: + compressed = (compressor.compress(message) + + compressor.flush(flush_mode)) + assert compressed.endswith(b"\x00\x00\xff\xff") + assert decompressor.decompress(compressed) == message + + @pytest.mark.parametrize("flush_mode", [isal_zlib.Z_SYNC_FLUSH, isal_zlib.Z_FULL_FLUSH]) @pytest.mark.parametrize(["strategy", "btype"], diff --git a/tests/test_dictionary_header.py b/tests/test_dictionary_header.py index 028b88c7..c2adbd1a 100644 --- a/tests/test_dictionary_header.py +++ b/tests/test_dictionary_header.py @@ -11,7 +11,8 @@ @pytest.mark.parametrize('wbits', [9, 15, -15]) @pytest.mark.parametrize('dictionary_size', [11, 32768, 40000]) @pytest.mark.parametrize( - 'first_operation', ['compress', 'empty', 'sync', 'finish']) + 'first_operation', + ['compress', 'empty', 'sync', 'repeated sync', 'finish']) def test_dictionary_cross_decode(level, wbits, dictionary_size, first_operation): dictionary = random.Random(21).randbytes(dictionary_size) @@ -22,6 +23,9 @@ def test_dictionary_cross_decode(level, wbits, dictionary_size, packed += compressor.compress(b'') elif first_operation == 'sync': packed += compressor.flush(isal_zlib.Z_SYNC_FLUSH) + elif first_operation == 'repeated sync': + packed += compressor.flush(isal_zlib.Z_SYNC_FLUSH) + assert compressor.flush(isal_zlib.Z_SYNC_FLUSH) == b'' elif first_operation == 'finish': data = b'' for pos in range(0, len(data), 17): diff --git a/tests/test_igzip.py b/tests/test_igzip.py index 1c691c2b..1b25284b 100644 --- a/tests/test_igzip.py +++ b/tests/test_igzip.py @@ -795,6 +795,19 @@ def test_small_writes_tell_and_flush(): assert gzip.decompress(output.getvalue()) == b"abcdefgh" +def test_repeated_flush_writes_nothing(): + output = io.BytesIO() + with igzip.IGzipFile(fileobj=output, mode="wb") as gzip_file: + gzip_file.write(b"abc") + gzip_file.flush() + size = len(output.getvalue()) + gzip_file.flush() + assert len(output.getvalue()) == size + gzip_file.write(b"def") + gzip_file.flush() + assert gzip.decompress(output.getvalue()) == b"abcdef" + + @pytest.mark.skipif(sys.platform != "linux", reason="bash required") def test_no_broken_pipe(): subprocess.run(