Skip to content

Commit 3d5cdd0

Browse files
committed
Address review feedback: fix bugs, imports, and documentation
- Fix NotImplemented -> NotImplementedError in test_types.py (was raising the singleton constant instead of the exception class) - Remove unused DoubleType import in unit/test_types.py - Replace fragile sys.path.insert(0, '.') with __file__-relative path resolution in vector_deserialize.py - Fix ShortType test data truncation: use modulo instead of clamping vector length to avoid producing fewer elements than vector_size - Add forward-looking note to DesVectorType variable-size subtype test - Expand Cython benchmark docstring to explain skip behavior
1 parent 3775e35 commit 3d5cdd0

3 files changed

Lines changed: 96 additions & 50 deletions

File tree

benchmarks/vector_deserialize.py

Lines changed: 91 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,22 @@
2525
Run with: python benchmarks/vector_deserialize.py
2626
"""
2727

28+
import os
2829
import sys
2930
import time
3031
import struct
3132

32-
# Add parent directory to path
33-
sys.path.insert(0, '.')
33+
# Add project root to path so the benchmark can be run from any directory
34+
sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), ".."))
3435

3536
from cassandra.cqltypes import FloatType, DoubleType, Int32Type, LongType, ShortType
36-
from cassandra.marshal import float_pack, double_pack, int32_pack, int64_pack, int16_pack
37+
from cassandra.marshal import (
38+
float_pack,
39+
double_pack,
40+
int32_pack,
41+
int64_pack,
42+
int16_pack,
43+
)
3744

3845

3946
def create_test_data(vector_size, element_type):
@@ -51,13 +58,13 @@ def create_test_data(vector_size, element_type):
5158
values = list(range(vector_size))
5259
pack_fn = int64_pack
5360
elif element_type == ShortType:
54-
values = list(range(min(vector_size, 32767)))
61+
values = [i % 32767 for i in range(vector_size)]
5562
pack_fn = int16_pack
5663
else:
5764
raise ValueError(f"Unsupported element type: {element_type}")
5865

5966
# Serialize the vector
60-
serialized = b''.join(pack_fn(v) for v in values)
67+
serialized = b"".join(pack_fn(v) for v in values)
6168

6269
return serialized, values
6370

@@ -83,16 +90,26 @@ def benchmark_struct_optimization(vector_type, serialized_data, iterations=10000
8390
subtype = vector_type.subtype
8491

8592
# Determine format string - subtype is a class, use identity or issubclass
86-
if subtype is FloatType or (isinstance(subtype, type) and issubclass(subtype, FloatType)):
87-
format_str = f'>{vector_size}f'
88-
elif subtype is DoubleType or (isinstance(subtype, type) and issubclass(subtype, DoubleType)):
89-
format_str = f'>{vector_size}d'
90-
elif subtype is Int32Type or (isinstance(subtype, type) and issubclass(subtype, Int32Type)):
91-
format_str = f'>{vector_size}i'
92-
elif subtype is LongType or (isinstance(subtype, type) and issubclass(subtype, LongType)):
93-
format_str = f'>{vector_size}q'
94-
elif subtype is ShortType or (isinstance(subtype, type) and issubclass(subtype, ShortType)):
95-
format_str = f'>{vector_size}h'
93+
if subtype is FloatType or (
94+
isinstance(subtype, type) and issubclass(subtype, FloatType)
95+
):
96+
format_str = f">{vector_size}f"
97+
elif subtype is DoubleType or (
98+
isinstance(subtype, type) and issubclass(subtype, DoubleType)
99+
):
100+
format_str = f">{vector_size}d"
101+
elif subtype is Int32Type or (
102+
isinstance(subtype, type) and issubclass(subtype, Int32Type)
103+
):
104+
format_str = f">{vector_size}i"
105+
elif subtype is LongType or (
106+
isinstance(subtype, type) and issubclass(subtype, LongType)
107+
):
108+
format_str = f">{vector_size}q"
109+
elif subtype is ShortType or (
110+
isinstance(subtype, type) and issubclass(subtype, ShortType)
111+
):
112+
format_str = f">{vector_size}h"
96113
else:
97114
return None, None, None
98115

@@ -118,16 +135,26 @@ def benchmark_numpy_optimization(vector_type, serialized_data, iterations=10000)
118135
subtype = vector_type.subtype
119136

120137
# Determine dtype
121-
if subtype is FloatType or (isinstance(subtype, type) and issubclass(subtype, FloatType)):
122-
dtype = '>f4'
123-
elif subtype is DoubleType or (isinstance(subtype, type) and issubclass(subtype, DoubleType)):
124-
dtype = '>f8'
125-
elif subtype is Int32Type or (isinstance(subtype, type) and issubclass(subtype, Int32Type)):
126-
dtype = '>i4'
127-
elif subtype is LongType or (isinstance(subtype, type) and issubclass(subtype, LongType)):
128-
dtype = '>i8'
129-
elif subtype is ShortType or (isinstance(subtype, type) and issubclass(subtype, ShortType)):
130-
dtype = '>i2'
138+
if subtype is FloatType or (
139+
isinstance(subtype, type) and issubclass(subtype, FloatType)
140+
):
141+
dtype = ">f4"
142+
elif subtype is DoubleType or (
143+
isinstance(subtype, type) and issubclass(subtype, DoubleType)
144+
):
145+
dtype = ">f8"
146+
elif subtype is Int32Type or (
147+
isinstance(subtype, type) and issubclass(subtype, Int32Type)
148+
):
149+
dtype = ">i4"
150+
elif subtype is LongType or (
151+
isinstance(subtype, type) and issubclass(subtype, LongType)
152+
):
153+
dtype = ">i8"
154+
elif subtype is ShortType or (
155+
isinstance(subtype, type) and issubclass(subtype, ShortType)
156+
):
157+
dtype = ">i2"
131158
else:
132159
return None, None, None
133160

@@ -144,7 +171,12 @@ def benchmark_numpy_optimization(vector_type, serialized_data, iterations=10000)
144171

145172

146173
def benchmark_cython_deserializer(vector_type, serialized_data, iterations=10000):
147-
"""Benchmark Cython DesVectorType deserializer."""
174+
"""Benchmark Cython DesVectorType deserializer.
175+
176+
This benchmark requires the Cython deserializers extension to be compiled.
177+
When the extension is not available, or the type does not have a dedicated
178+
DesVectorType deserializer, the benchmark is silently skipped (returns None).
179+
"""
148180
try:
149181
from cassandra.deserializers import find_deserializer
150182
except ImportError:
@@ -156,7 +188,7 @@ def benchmark_cython_deserializer(vector_type, serialized_data, iterations=10000
156188
deserializer = find_deserializer(vector_type)
157189

158190
# Check if we got the Cython deserializer
159-
if deserializer.__class__.__name__ != 'DesVectorType':
191+
if deserializer.__class__.__name__ != "DesVectorType":
160192
return None, None, None
161193

162194
start = time.perf_counter()
@@ -193,15 +225,18 @@ def verify_results(expected, *results):
193225

194226
def run_benchmark_suite(vector_size, element_type, type_name, iterations=10000):
195227
"""Run complete benchmark suite for a given vector configuration."""
196-
print(f"\n{'='*80}")
228+
print(f"\n{'=' * 80}")
197229
print(f"Benchmark: Vector<{type_name}, {vector_size}>")
198-
print(f"{'='*80}")
230+
print(f"{'=' * 80}")
199231
print(f"Iterations: {iterations:,}")
200232

201233
# Create test data
202234
from cassandra.cqltypes import lookup_casstype
203-
cass_typename = f'org.apache.cassandra.db.marshal.{element_type.__name__}'
204-
vector_typename = f'org.apache.cassandra.db.marshal.VectorType({cass_typename}, {vector_size})'
235+
236+
cass_typename = f"org.apache.cassandra.db.marshal.{element_type.__name__}"
237+
vector_typename = (
238+
f"org.apache.cassandra.db.marshal.VectorType({cass_typename}, {vector_size})"
239+
)
205240
vector_type = lookup_casstype(vector_typename)
206241

207242
serialized_data, expected_values = create_test_data(vector_size, element_type)
@@ -216,41 +251,51 @@ def run_benchmark_suite(vector_size, element_type, type_name, iterations=10000):
216251
# 1. Current implementation (baseline)
217252
print("1. Current implementation (baseline)...")
218253
elapsed, per_op, result_current = benchmark_current_implementation(
219-
vector_type, serialized_data, iterations)
254+
vector_type, serialized_data, iterations
255+
)
220256
results.append(result_current)
221257
print(f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs")
222258
baseline_time = per_op
223259

224260
# 2. Struct optimization
225261
print("2. Python struct.unpack optimization...")
226262
elapsed, per_op, result_struct = benchmark_struct_optimization(
227-
vector_type, serialized_data, iterations)
263+
vector_type, serialized_data, iterations
264+
)
228265
results.append(result_struct)
229266
if per_op is not None:
230267
speedup = baseline_time / per_op
231-
print(f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x")
268+
print(
269+
f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x"
270+
)
232271
else:
233272
print(" Not applicable for this type")
234273

235274
# 3. Numpy with tolist()
236275
print("3. Numpy frombuffer + tolist()...")
237276
elapsed, per_op, result_numpy = benchmark_numpy_optimization(
238-
vector_type, serialized_data, iterations)
277+
vector_type, serialized_data, iterations
278+
)
239279
results.append(result_numpy)
240280
if per_op is not None:
241281
speedup = baseline_time / per_op
242-
print(f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x")
282+
print(
283+
f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x"
284+
)
243285
else:
244286
print(" Numpy not available")
245287

246288
# 4. Cython deserializer
247289
print("4. Cython DesVectorType deserializer...")
248290
elapsed, per_op, result_cython = benchmark_cython_deserializer(
249-
vector_type, serialized_data, iterations)
291+
vector_type, serialized_data, iterations
292+
)
250293
if per_op is not None:
251294
results.append(result_cython)
252295
speedup = baseline_time / per_op
253-
print(f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x")
296+
print(
297+
f" Total: {elapsed:.4f}s, Per-op: {per_op:.2f} μs, Speedup: {speedup:.2f}x"
298+
)
254299
else:
255300
print(" Cython deserializers not available")
256301

@@ -269,30 +314,28 @@ def main():
269314
# Pin to single CPU core for consistent measurements
270315
try:
271316
import os
317+
272318
os.sched_setaffinity(0, {0}) # Pin to CPU core 0
273319
print("Pinned to CPU core 0 for consistent measurements")
274320
except (AttributeError, OSError) as e:
275321
print(f"Could not pin to single core: {e}")
276322
print("Running without CPU affinity...")
277323

278-
print("="*80)
324+
print("=" * 80)
279325
print("VectorType Deserialization Performance Benchmark")
280-
print("="*80)
326+
print("=" * 80)
281327

282328
# Test configurations: (vector_size, element_type, type_name, iterations)
283329
test_configs = [
284330
# Small vectors
285331
(3, FloatType, "float", 50000),
286332
(4, FloatType, "float", 50000),
287-
288333
# Medium vectors (common in ML)
289334
(128, FloatType, "float", 10000),
290335
(384, FloatType, "float", 10000),
291-
292336
# Large vectors (embeddings)
293337
(768, FloatType, "float", 5000),
294338
(1536, FloatType, "float", 2000),
295-
296339
# Other types (smaller iteration counts)
297340
(128, DoubleType, "double", 10000),
298341
(768, DoubleType, "double", 5000),
@@ -308,16 +351,16 @@ def main():
308351
summary.append((f"Vector<{type_name}, {vector_size}>", baseline))
309352

310353
# Print summary
311-
print("\n" + "="*80)
354+
print("\n" + "=" * 80)
312355
print("SUMMARY - Current Implementation Performance")
313-
print("="*80)
356+
print("=" * 80)
314357
for config, baseline_time in summary:
315358
print(f"{config:30s}: {baseline_time:8.2f} μs")
316359

317-
print("\n" + "="*80)
360+
print("\n" + "=" * 80)
318361
print("Benchmark complete!")
319-
print("="*80)
362+
print("=" * 80)
320363

321364

322-
if __name__ == '__main__':
365+
if __name__ == "__main__":
323366
main()

tests/integration/standard/test_types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ def test_can_insert_tuples_all_collection_datatypes(self):
723723

724724
# make sure we're testing all non primitive data types in the future
725725
if set(COLLECTION_TYPES) != set(["tuple", "list", "map", "set"]):
726-
raise NotImplemented(
726+
raise NotImplementedError(
727727
"Missing datatype not implemented: {}".format(
728728
set(COLLECTION_TYPES) - set(["tuple", "list", "map", "set"])
729729
)

tests/unit/test_types.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,10 @@ def test_vector_cython_deserializer_variable_size_subtype(self):
747747
and should raise ValueError from DesVectorType._deserialize_generic.
748748
The pure Python VectorType.deserialize handles these correctly.
749749
750+
Note: This test is forward-looking — it validates the Cython deserializer
751+
that is introduced in a companion PR. The skipTest guards below ensure
752+
the test is silently skipped when the extension is not yet compiled.
753+
750754
@since 3.x
751755
@expected_result Cython deserializer raises ValueError for variable-size subtypes;
752756
pure Python path correctly deserializes them
@@ -787,7 +791,6 @@ def test_vector_numpy_large_deserialization(self):
787791
@test_category data_types:vector
788792
"""
789793
import struct
790-
from cassandra.cqltypes import DoubleType
791794

792795
vector_size = 64 # >= 32 threshold for numpy path
793796

0 commit comments

Comments
 (0)