Uh oh!
There was an error while loading. Please reload this page.
(improvement) Optimize Cython deserialization primitives and add VectorType Cython deserializer (substantial - 11x-30x speedup mainly via DesVectorType.deserialize_bytes with Cython) - #732
Conversation
34dd41e to
9b7697cCompareImportant Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
mykaul
commented
Jul 29, 2026
Rebased onto 1. 2. 3. Circular import spuriously breaking Also updated Testing: rebuilt the Cython extensions locally and ran the full unit suite: No review threads existed on this PR to resolve. Still a draft as intended. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR improves Cython-based deserialization performance and adds a dedicated Cython deserializer for VectorType, targeting large speedups for numeric vector decoding.
Changes:
- Replace varint hex-string conversion with
int.from_bytes(..., signed=True)in both Python and Cython marshal paths. - Introduce
DesVectorTypewith C-level fast paths (plus optional NumPy path) and updatefind_deserializer()routing for vectors. - Tighten buffer handling by replacing
slice_buffer()withfrom_ptr_and_size()and adding explicit bounds validation in several deserializers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_types.py | Adds unit tests covering DesVectorType selection and vector subtype routing/fallback behavior. |
| cassandra/marshal.py | Optimizes Python varint_unpack() using int.from_bytes. |
| cassandra/ioutils.pyx | Optimizes read_int() using memcpy + ntohl for alignment-safe decoding. |
| cassandra/deserializers.pyx | Adds DesVectorType, NumPy fast-path, subtype-aware routing, and additional bounds checks / buffer slicing changes. |
| cassandra/cython_marshal.pyx | Optimizes numeric unpacking using ntohs/ntohl + int.from_bytes and adds a float-specific swap path. |
| cassandra/buffer.pxd | Removes slice_buffer() and documents/implements from_ptr_and_size() including negative-size sentinels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…y large vector deserialization Add test_vector_cython_deserializer_variable_size_subtype to verify find_deserializer()'s current dispatch for a VectorType with a variable-size subtype (e.g. UTF8Type). cassandra.deserializers has no dedicated Des* class for VectorType today, and VectorType is not a subclass of any of the collection types find_deserializer() special-cases, so it always falls through to GenericDeserializer -- regardless of whether the subtype has a fixed serialized size or not. GenericDeserializer simply delegates to the pure Python VectorType.deserialize(), which correctly round-trips variable-size subtypes; the test asserts both the dispatch and the round-trip. A Cython fast-path deserializer for VectorType (DesVectorType), with dispatch that fast-paths fixed-size subtypes while leaving variable-size subtypes on GenericDeserializer, is being developed in companion PRs (scylladb#689, scylladb#732). Neither has merged, so DesVectorType does not exist on this branch or on master; this test intentionally does not depend on it. Once one of those PRs lands, this test should be revisited to also assert the Cython dispatch/behavior for fixed-size subtypes. Add test_vector_numpy_large_deserialization to exercise VectorType deserialization for vectors with >= 32 elements across all supported fixed-size numeric types (float, double, int32, int64). VectorType has no numpy-specific fast path today; the test name/threshold is forward-looking and documents that, guarding correctness of the existing implementation at representative embedding sizes in the meantime. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
…y large vector deserialization Add test_vector_cython_deserializer_variable_size_subtype to verify find_deserializer()'s current dispatch for a VectorType with a variable-size subtype (e.g. UTF8Type). cassandra.deserializers has no dedicated Des* class for VectorType today, and VectorType is not a subclass of any of the collection types find_deserializer() special-cases, so it always falls through to GenericDeserializer -- regardless of whether the subtype has a fixed serialized size or not. GenericDeserializer simply delegates to the pure Python VectorType.deserialize(), which correctly round-trips variable-size subtypes; the test asserts both the dispatch and the round-trip. A Cython fast-path deserializer for VectorType (DesVectorType), with dispatch that fast-paths fixed-size subtypes while leaving variable-size subtypes on GenericDeserializer, is being developed in companion PRs (scylladb#689, scylladb#732). Neither has merged, so DesVectorType does not exist on this branch or on master; this test intentionally does not depend on it. Once one of those PRs lands, this test should be revisited to also assert the Cython dispatch/behavior for fixed-size subtypes. Add test_vector_numpy_large_deserialization to exercise VectorType deserialization for vectors with >= 32 elements across all supported fixed-size numeric types (float, double, int32, int64). VectorType has no numpy-specific fast path today; the test name/threshold is forward-looking and documents that, guarding correctness of the existing implementation at representative embedding sizes in the meantime. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
…y large vector deserialization Add test_vector_cython_deserializer_variable_size_subtype to verify find_deserializer()'s current dispatch for a VectorType with a variable-size subtype (e.g. UTF8Type). cassandra.deserializers has no dedicated Des* class for VectorType today, and VectorType is not a subclass of any of the collection types find_deserializer() special-cases, so it always falls through to GenericDeserializer -- regardless of whether the subtype has a fixed serialized size or not. GenericDeserializer simply delegates to the pure Python VectorType.deserialize(), which correctly round-trips variable-size subtypes; the test asserts both the dispatch and the round-trip. A Cython fast-path deserializer for VectorType (DesVectorType), with dispatch that fast-paths fixed-size subtypes while leaving variable-size subtypes on GenericDeserializer, is being developed in companion PRs (scylladb#689, scylladb#732). Neither has merged, so DesVectorType does not exist on this branch or on master; this test intentionally does not depend on it. Once one of those PRs lands, this test should be revisited to also assert the Cython dispatch/behavior for fixed-size subtypes. Add test_vector_numpy_large_deserialization to exercise VectorType deserialization for vectors with >= 32 elements across all supported fixed-size numeric types (float, double, int32, int64). VectorType has no numpy-specific fast path today; the test name/threshold is forward-looking and documents that, guarding correctness of the existing implementation at representative embedding sizes in the meantime. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
….from_bytes Performance improvements to serialization/deserialization hot paths: 1. unpack_num(): Use ntohs()/ntohl() for 16-bit and 32-bit integer types instead of byte-by-byte swapping loop. These compile to single bswap instructions on x86, providing more predictable performance. 2. read_int(): Simplify to use ntohl() directly instead of going through unpack_num() with a temporary Buffer. 3. varint_unpack(): Replace hex string conversion with int.from_bytes(). This eliminates string allocations and provides 4-18x speedup for the function itself (larger gains for longer varints). 4. Remove slice_buffer() and replaced with direct assignment 5. _unpack_len() is now implemented similar to read_int() Also removes unused 'start' and 'end' variables from unpack_num(). End-to-end benchmark shows ~4-5% improvement in row throughput. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
…helpers Add buffer bounds validation to Cython deserializers for safety against malformed buffers, refactor to use from_ptr_and_size() helper consistently, and add float ntohl() specialization for consistency with int32/int16 paths. Changes: - subelem(): Add CQL protocol-compliant value handling (NULL/-1, not-set/-2, invalid/<-2) with bounds checking - _unpack_len(): Add bounds check and use memcpy for alignment safety - DesTupleType: Add defensive bounds checking for tuple item lengths - DesCompositeType: Add bounds validation for composite element lengths - Refactor 4 locations to use from_ptr_and_size() instead of manual Buffer field assignment - Add float branch to unpack_num(): reinterpret bits as uint32, ntohl(), reinterpret back (consistent with int16/int32 intrinsic paths) - Add from_ptr_and_size() declaration to buffer.pxd Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
…izer Addded DesVectorType Cython deserializer with C-level optimizations for improved performance in row parsing for vectors. The deserializer uses: - Direct C byte swapping (ntohl) for numeric types - Memory operations without Python object overhead - Unified numpy path for large vectors (≥32 elements) - Hand-written fast paths for small vectors (<32 elements) Only float/double/int32/bigint subtypes get the fast C-level path: these are the only VectorType subtypes that are genuinely fixed-width on the wire (they have a real serial_size() override). Every other subtype -- including variable-length/vint-prefixed ones like text, varint, and smallint (ShortType has no serial_size() override and is vint-prefixed per element in a vector, not a raw 2-byte value) -- falls back to GenericDeserializer, which delegates to the pure-Python VectorType.deserialize() and handles them correctly. find_deserializer() is subtype-aware for VectorType so it never routes an unsupported subtype to the fast path, where it would otherwise surface as an uncaught ValueError during row parsing. Also avoid importing cassandra.cython_deps from this module: cython_deps imports cassandra.row_parser, which imports this module for make_deserializers(), so importing cython_deps back from here closes an import cycle. Depending on import order this makes Python see cython_deps as partially initialized while resolving HAVE_NUMPY, raising ImportError, which cython_deps' own try/except silently turns into HAVE_CYTHON = False even though the Cython extensions built and imported fine. Do our own independent numpy-availability check instead. Performance improvements: - Small vectors (3-4 elements): 4.4-4.7x faster - Medium vectors (128 elements): 1.0-1.5x faster - Large vectors (384-1536 elements): 0.9-1.0x (marginal) (measured for the fixed-width fast path subtypes) The Cython deserializer is automatically used by the row parser when available via find_deserializer(). Includes unit tests and benchmark code. Follow-up commits will try to get Numpy arrays, and perhaps more. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
The 'values' list was allocated but never used — the method builds results directly into a pre-allocated tuple via tuple_set(res, i, item). Removes one unnecessary list allocation per tuple deserialization.
Summary
Optimize foundational Cython byte-unpacking and add a dedicated VectorType Cython deserializer.
Commits (4, squashed from 7)
1. Optimize Cython byte unpacking with ntohs/ntohl and int.from_bytes
unpack_num()withntohs()/ntohl()intrinsics for 16/32-bit types (compiles to singlebswapon x86)varint_unpack()hex-string-based conversion withint.from_bytes(term, 'big', signed=True)— 7.7x speedupread_int()to direct pointer cast +ntohl()slice_buffer(), replace all call sites withfrom_ptr_and_size()#ifdef _WIN32forwinsock2.hvsarpa/inet.h2. Optimize float deserialization with ntohl() intrinsic
uint32_t, applyntohl(), reinterpret back to floatfrom_ptr_and_size()helper consistentlysubelem(), bounds checks in_unpack_len(),DesTupleType,DesCompositeType)3. Optimize VectorType deserialization with Cython deserializer
DesVectorTypeclass with specialized deserialization methods:_deserialize_float(): C-levelmemcpy+ntohl+ pointer-cast (no Python dispatch per element)_deserialize_double()/_deserialize_int64(): 8-byte manual byte-swap_deserialize_int32():memcpy+ntohl+ cast_deserialize_int16():ntohscastfind_deserializer()for the Cython row parser4. Remove dead
values = []in DesTupleType.deserializevalueslist was allocated but never used — results built directly into pre-allocated tuple viatuple_set()Benchmark Results
All benchmarks:
min(timeit.repeat(number=N, repeat=5)), per-call nanoseconds.Machine: idle Linux workstation, Cython extensions compiled.
Primitives (via
CqlType.deserialize())The ntohs/ntohl change replaces a byte-swap loop that was already fast for 2/4-byte types. The big win is
varint_unpack()whereint.from_bytes()replaces hex-string conversion.VectorType — Python path (via
VectorType.deserialize())VectorType — Cython path (via
DesVectorType.deserialize_bytes())The Cython
DesVectorTypeis used by the Cython row parser (find_deserializer()), bypassing the PythonVectorType.deserialize()entirely:Unit tests
640 passed, 49 skipped (baseline: 645 passed, 43 skipped on master).