Address LZ4 review feedback

Author: mykaul

benchmarks/bench_lz4.py

@@ -79,8 +79,8 @@ def py_lz4_decompress(byts):
 def make_payload(size):
     """Generate a pseudo-realistic compressible payload."""
     # Mix of repetitive and random-ish bytes to simulate CQL result rows.
-    chunk = (b"row_value_" + os.urandom(6)) * (size // 16 + 1)
-    return chunk[:size]
+    blocks = [b"row_value_" + os.urandom(6) for _ in range(size // 16 + 1)]
+    return b"".join(blocks)[:size]
 
 
 def bench(label, func, arg, inner=INNER, repeat=REPEAT):

cassandra/cython_lz4.pyx

@@ -55,23 +55,22 @@ cdef extern from *:
 # theoretical maximum is ~2 GiB.  We use 256 MiB as a practical upper
 # bound (matching the server's default frame size limit) to avoid
 # accidentally allocating multi-GiB buffers on corrupt headers.
-DEF MAX_DECOMPRESSED_LENGTH = 268435456  # 256 MiB
+cdef enum:
+    MAX_DECOMPRESSED_LENGTH = 268435456  # 256 MiB
 
 # LZ4_MAX_INPUT_SIZE from lz4.h — the LZ4 C API uses C int (32-bit
 # signed) for sizes, so we must reject Python bytes objects that
 # exceed this before casting Py_ssize_t down to int.
-DEF LZ4_MAX_INPUT_SIZE = 0x7E000000  # 2 113 929 216 bytes
+cdef enum:
+    LZ4_MAX_INPUT_SIZE = 0x7E000000  # 2 113 929 216 bytes
 
 # Maximum LZ4_compressBound value for which we use a fixed-size buffer on
-# the C stack instead of malloc.  128 KiB is well within the default
-# 8 MiB thread stack size (POSIX) / 1 MiB (Windows) and covers CQL frames
-# up to ~127 KiB uncompressed — the vast majority of real traffic.  Larger
-# frames fall back to heap allocation.  A plain fixed-size array is used
-# (rather than alloca()) because alloca() is declared in <alloca.h>, which
-# does not exist on Windows/MSVC (it ships _alloca() in <malloc.h> instead
-# with subtly different semantics) — a fixed-size array avoids that
-# platform split entirely while remaining just as cheap.
-DEF STACK_ALLOC_THRESHOLD = 131072  # 128 KiB
+# the C stack instead of malloc.  16 KiB covers common CQL frames while
+# keeping the per-call stack frame small.  Larger frames fall back to heap
+# allocation.  A plain fixed-size array is used instead of alloca(), which
+# is not portable across Windows/MSVC and POSIX.
+cdef enum:
+    STACK_ALLOC_THRESHOLD = 16384  # 16 KiB
 
 
 cdef extern from "lz4.h":

conanfile.py

@@ -26,7 +26,8 @@ def generate(self) -> None:
         build_req = self._conanfile.dependencies.build  # tool_requires
         test_req = self._conanfile.dependencies.test
 
-        content_buffer = ""
+        include_dirs = []
+        library_dirs = []
 
         # Filter the build_requires not activated for any requirement
         dependencies = [tup for tup in list(host_req.items()) + list(build_req.items()) + list(test_req.items()) if not tup[0].build]
@@ -37,17 +38,19 @@ def generate(self) -> None:
                 continue
             include_dir = Path(dep.package_folder) / 'include'
             package_dir = Path(dep.package_folder) / 'lib'
-            content_buffer += json.dumps(dict(include_dirs=str(include_dir), library_dirs=str(package_dir)))
+            include_dirs.append(str(include_dir))
+            library_dirs.append(str(package_dir))
 
-        save(self._conanfile, CONAN_COMMANDLINE_FILENAME, content_buffer)
+        content = json.dumps(dict(include_dirs=include_dirs, library_dirs=library_dirs))
+        save(self._conanfile, CONAN_COMMANDLINE_FILENAME, content)
         self._conanfile.output.info(f"Generated {CONAN_COMMANDLINE_FILENAME}")
 
 
 class python_driverConan(ConanFile):
     win_bash = False
 
     settings = "os", "compiler", "build_type", "arch"
-    requires = "libev/4.33"
+    requires = "libev/4.33", "lz4/1.9.4"
 
     def layout(self):
         basic_layout(self)

setup.py

@@ -155,8 +155,8 @@ def eval_env_var_as_array(varname):
 conan_envfile = Path(__file__).parent / 'build-release/conan/conandeps.env'
 if conan_envfile.exists():
     conan_paths = json.loads(conan_envfile.read_text())
-    libev_includes.extend([conan_paths.get('include_dirs')])
-    libev_libdirs.extend([conan_paths.get('library_dirs')])
+    libev_includes.extend(conan_paths.get('include_dirs', []))
+    libev_libdirs.extend(conan_paths.get('library_dirs', []))
 
 libev_ext = Extension('cassandra.io.libevwrapper',
                       sources=['cassandra/io/libevwrapper.c'],

tests/unit/cython/test_cython_lz4.py

@@ -74,11 +74,11 @@ def test_round_trip_64kb(self):
         self.assertEqual(lz4_decompress(lz4_compress(data)), data)
 
     def test_round_trip_heap_buffer(self):
-        """Inputs above the 128 KiB stack threshold use the malloc path."""
-        # LZ4_compressBound(131072) = 131072 + 131072/255 + 16 = 131602,
+        """Inputs above the 16 KiB stack threshold use the malloc path."""
+        # LZ4_compressBound(16384) = 16384 + 16384/255 + 16 = 16464,
         # which exceeds STACK_ALLOC_THRESHOLD, so lz4_compress must fall
         # back to the heap buffer and free it on success.
-        data = os.urandom(131072)
+        data = os.urandom(16384)
         self.assertEqual(lz4_decompress(lz4_compress(data)), data)
 
     def test_round_trip_empty(self):
@@ -207,6 +207,17 @@ def test_cross_compat_64kb(self):
     def test_cross_compat_empty(self):
         self._check_cross_compat(b"")
 
+    def test_connection_prefers_cython_codec(self):
+        """The connection layer selects the Cython codec when present."""
+        from cassandra import connection
+
+        self.assertIs(connection.locally_supported_compressions['lz4'][0],
+                      lz4_compress)
+        self.assertIs(connection.locally_supported_compressions['lz4'][1],
+                      lz4_decompress)
+        self.assertIs(connection.segment_codec_lz4.compressor, lz4_compress)
+        self.assertIs(connection.segment_codec_lz4.decompressor, lz4_decompress)
+
 
 if __name__ == "__main__":
     unittest.main()