Merge pull request #106 from pycompression/fasterungzip

Improve GzipReader compression rates.

Author: rhpvorderman

CHANGELOG.rst

@@ -33,6 +33,10 @@ that PyPy is no longer supported.
   ``isal_zlib.IsalError`` has been removed.
 + The base class for ``isal_zlib.error`` and ``igzip_lib.IsalError`` is now
   ``Exception`` instead of ``OSError``.
++ GzipReader now uses larger input and output buffers (128k) by default and
+  IgzipDecompressor.decompress has been updated to allocate ``maxsize`` buffers
+  when these are of reasonable size, instead of growing the buffer to maxsize
+  on every call. This has improved gzip decompression speeds by 7%.
 + ISA-L library version variables are now available on windows as well.
 + Wheels are now always build with nasm for the x86 architecture. Previously
   yasm was used for Linux and MacOS due to build issues that have since been

benchmark.py renamed to benchmark_scripts/benchmark.py

@@ -8,7 +8,7 @@
 
 from isal import igzip, isal_zlib  # noqa: F401 used in timeit strings
 
-DATA_DIR = Path(__file__).parent / "tests" / "data"
+DATA_DIR = Path(__file__).parent.parent / "tests" / "data"
 COMPRESSED_FILE = DATA_DIR / "test.fastq.gz"
 with gzip.open(str(COMPRESSED_FILE), mode="rb") as file_h:
     data = file_h.read()

benchmark_scripts/gzipread128kblocks.py

@@ -0,0 +1,9 @@
+import sys
+
+from isal import igzip
+
+with igzip.open(sys.argv[1], "rb") as gzip_file:
+    while True:
+        block = gzip_file.read(128 * 1024)
+        if not block:
+            break

benchmark_scripts/gzipreadlines.py

@@ -0,0 +1,7 @@
+import sys
+
+from isal import igzip
+
+with igzip.open(sys.argv[1], "rb") as gzip_file:
+    for line in gzip_file:
+        pass

profile_igzipreader.py renamed to benchmark_scripts/profile_igzipreader.py

@@ -5,10 +5,9 @@
 
 
 def main():
-    igzip.READ_BUFFER_SIZE = 32 * 1024
     with igzip.open(sys.argv[1], mode="rb") as gzip_h:
         while True:
-            block = gzip_h.read(32*1024)
+            block = gzip_h.read(128*1024)
             if block == b"":
                 return
 

src/isal/igzip.py

@@ -49,7 +49,9 @@
 
 #: The amount of data that is read in at once when decompressing a file.
 #: Increasing this value may increase performance.
-READ_BUFFER_SIZE = io.DEFAULT_BUFFER_SIZE
+#: 128K is also the size used by pigz and cat to read files from the
+# filesystem.
+READ_BUFFER_SIZE = 128 * 1024
 
 FTEXT, FHCRC, FEXTRA, FNAME, FCOMMENT = 1, 2, 4, 8, 16
 
@@ -164,7 +166,7 @@ def __init__(self, filename=None, mode=None,
                                                   0)
         if self.mode == gzip.READ:
             raw = _IGzipReader(self.fileobj)
-            self._buffer = io.BufferedReader(raw)
+            self._buffer = io.BufferedReader(raw, buffer_size=READ_BUFFER_SIZE)
 
     def __repr__(self):
         s = repr(self.fileobj)
@@ -247,12 +249,6 @@ def __init__(self, fp):
         self._new_member = True
         self._last_mtime = None
 
-    def _add_read_data(self, data):
-        # Use faster isal crc32 calculation and update the stream size in place
-        # compared to CPython gzip
-        self._crc = isal_zlib.crc32(data, self._crc)
-        self._stream_size += len(data)
-
     def read(self, size=-1):
         if size < 0:
             return self.readall()
@@ -300,7 +296,8 @@ def read(self, size=-1):
                 raise EOFError("Compressed file ended before the "
                                "end-of-stream marker was reached")
 
-        self._add_read_data(uncompress)
+        self._crc = isal_zlib.crc32(uncompress, self._crc)
+        self._stream_size += len(uncompress)
         self._pos += len(uncompress)
         return uncompress
 
@@ -451,7 +448,7 @@ def _argument_parser():
     # diminishing returns hit. _compression.BUFFER_SIZE = 8k. But 32K is about
     # ~6% faster.
     parser.add_argument("-b", "--buffer-size",
-                        default=128 * 1024, type=int,
+                        default=READ_BUFFER_SIZE, type=int,
                         help=argparse.SUPPRESS)
     return parser
 

src/isal/igzip_lib.c

@@ -121,12 +121,27 @@ decompress_buf(IgzipDecompressor *self, Py_ssize_t max_length)
        compare against max_length and PyBytes_GET_SIZE we declare it as
        signed */
     PyObject * RetVal = NULL;
-    Py_ssize_t obuflen = DEF_BUF_SIZE;
+    Py_ssize_t hard_limit;
+    
+    Py_ssize_t obuflen;
+    
     int err;
 
-    if (obuflen > max_length)
+    if (max_length < 0) {
+        hard_limit = PY_SSIZE_T_MAX;
+        obuflen = DEF_BUF_SIZE;
+    } else {
+        // Assume that decompressor is used in file decompression with a fixed
+        // block size of max_length. In that case we will reach max_length almost
+        // always (except at the end of the file). So it makes sense to allocate
+        // max_length.
+        hard_limit = max_length;
         obuflen = max_length;
-
+        if (obuflen > DEF_MAX_INITIAL_BUF_SIZE){
+            // Safeguard against memory overflow.
+            obuflen = DEF_MAX_INITIAL_BUF_SIZE;
+        }
+    }
 
     do {
         arrange_input_buffer(&(self->state.avail_in), &(self->avail_in_real));
@@ -136,7 +151,7 @@ decompress_buf(IgzipDecompressor *self, Py_ssize_t max_length)
                                                         &(self->state.next_out),
                                                         &RetVal,
                                                         obuflen,
-                                                        max_length);
+                                                        hard_limit);
             if (obuflen == -1){
                 PyErr_SetString(PyExc_MemoryError, 
                                 "Unsufficient memory for buffer allocation");
@@ -176,12 +191,6 @@ decompress(IgzipDecompressor *self, uint8_t *data, size_t len, Py_ssize_t max_le
     char input_buffer_in_use;
     PyObject *result;
 
-    Py_ssize_t hard_limit;
-    if (max_length < 0) {
-        hard_limit = PY_SSIZE_T_MAX;
-    } else {
-        hard_limit = max_length;
-    }
     /* Prepend unconsumed input if necessary */
     if (self->state.next_in != NULL) {
         size_t avail_now, avail_total;
@@ -227,7 +236,7 @@ decompress(IgzipDecompressor *self, uint8_t *data, size_t len, Py_ssize_t max_le
         input_buffer_in_use = 0;
     }
 
-    result = decompress_buf(self, hard_limit);
+    result = decompress_buf(self, max_length);
     if(result == NULL) {
         self->state.next_in = NULL;
         return NULL;

src/isal/igzip_lib_impl.h

@@ -26,6 +26,7 @@
 
 /* Initial buffer size. */
 #define DEF_BUF_SIZE (16*1024)
+#define DEF_MAX_INITIAL_BUF_SIZE (16 * 1024 * 1024)
 #define ISAL_BEST_SPEED ISAL_DEF_MIN_LEVEL
 #define ISAL_BEST_COMPRESSION ISAL_DEF_MAX_LEVEL
 #define ISAL_DEFAULT_COMPRESSION 2

tests/test_gzip_compliance.py

@@ -572,13 +572,15 @@ def test_bytes_filename(self):
 
     def test_decompress_limited(self):
         """Decompressed data buffering should be limited"""
-        bomb = igzip.compress(b'\0' * int(2e6), compresslevel=3)
-        self.assertLess(len(bomb), io.DEFAULT_BUFFER_SIZE)
+        bomb_size = int(2e6)
+        self.assertLess(igzip.READ_BUFFER_SIZE, bomb_size)
+        bomb = gzip.compress(b'\0' * bomb_size, compresslevel=9)
+        self.assertLess(len(bomb), igzip.READ_BUFFER_SIZE)
 
         bomb = io.BytesIO(bomb)
         decomp = igzip.GzipFile(fileobj=bomb)
         self.assertEqual(decomp.read(1), b'\0')
-        max_decomp = 1 + io.DEFAULT_BUFFER_SIZE
+        max_decomp = 1 + igzip.READ_BUFFER_SIZE
         self.assertLessEqual(decomp._buffer.raw.tell(), max_decomp,
                              "Excessive amount of data was decompressed")
 

tox.ini

@@ -34,8 +34,8 @@ deps=flake8
      pytest
 skip_install=True
 commands =
-    flake8 src tests setup.py benchmark.py
-    mypy src/ tests
+    flake8 src tests setup.py benchmark_scripts
+    mypy src/ tests benchmark_scripts
 
 [testenv:twine_check]
 deps=build
@@ -67,19 +67,19 @@ commands=
 [testenv:benchmark-all]
 deps=
 commands=
-    python ./benchmark.py --all
+    python ./benchmark_scripts/benchmark.py --all
 
 [testenv:benchmark-functions]
 deps=
 commands=
-    python ./benchmark.py --functions
+    python ./benchmark_scripts/benchmark.py --functions
 
 [testenv:benchmark-gzip]
 deps=
 commands=
-    python ./benchmark.py --gzip
+    python ./benchmark_scripts/benchmark.py --gzip
 
 [testenv:benchmark-checksums]
 deps=
 commands=
-    python ./benchmark.py --checksums
+    python ./benchmark_scripts/benchmark.py --checksums