perf: optimize vector marshal/unmarshal for float32/float64/int32/int64 (Throughput: 75 MiB/s → 1.9 GiB/s (marshal), 106 MiB/s → 4.6 GiB/s (unmarshal) ) - #770
Draft
mykaul wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces high-performance, type-specialized marshal/unmarshal paths for common numeric vector element types to reduce reflect overhead and allocations in the gocql CQL codec layer.
Changes:
- Added fast paths in
marshalVector/unmarshalVectorfor[]float32,[]float64,[]int32,[]int64plus async.Pool-backed buffer helper for marshal-side reuse. - Added a generic-path preallocation helper (
vectorFixedElemSize+buf.Grow) to reducebytes.Buffergrowth for fixed-size element types. - Added extensive unit tests for vector behavior and expanded internal/public benchmarks for new int32/int64 vector cases and pooled scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| marshal.go | Adds vector fast paths, vector buffer pool helpers, and generic vector preallocation support. |
| marshal_vector_test.go | New comprehensive unit tests for vector fast paths, pooling helpers, and preallocation behavior. |
| vector_bench_test.go | Adds/extends internal benchmarks for pooled marshal and int32/int64 vector performance. |
| tests/bench/bench_vector_public_test.go | Extends public API benchmarks to cover int32/int64 vector marshal/unmarshal via gocql.Marshal/Unmarshal. |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
March 13, 2026 12:34
f6229cb to
c9ea393
Compare
There was a problem hiding this comment.
Pull request overview
Introduces type-specialized fast paths for vector marshal/unmarshal (float32/float64/int32/int64) in marshal.go to avoid reflect-heavy per-element encoding, plus expanded benchmarks and a new, comprehensive unit-test suite to validate correctness and performance characteristics.
Changes:
- Add fast-path vector marshal/unmarshal implementations using
encoding/binarybulk conversion and destination-slice reuse. - Add
sync.Pool-backed buffer helpers (getVectorBuf/putVectorBuf) and generic-path preallocation viavectorFixedElemSize. - Expand internal and public benchmarks; add a large new unit test file covering many vector edge cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
marshal.go |
Adds specialized vector marshal/unmarshal fast paths, pooling helpers, and generic-path preallocation. |
marshal_vector_test.go |
New unit tests for vector behavior (round-trip, byte-compat, slice reuse, pool behavior, etc.). |
vector_bench_test.go |
Adds pooled/write-path/round-trip benchmarks plus int32/int64 benchmark coverage. |
tests/bench/bench_vector_public_test.go |
Extends public API benchmarks to cover int32/int64 vectors. |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
March 13, 2026 18:03
c9ea393 to
79698fb
Compare
There was a problem hiding this comment.
Pull request overview
This PR accelerates CQL vector encoding/decoding in the GoCQL driver by introducing type-specialized marshal/unmarshal fast paths for common numeric vector element types, reducing reflection overhead and allocations on hot paths.
Changes:
- Add specialized marshal/unmarshal implementations for
[]float32,[]float64,[]int32,[]int64usingencoding/binary+ bit conversions, with unmarshal slice reuse. - Introduce
vectorBufPool(sync.Pool) helpers for reusable marshal buffers and add generic-path preallocation viavectorFixedElemSize()+bytes.Buffer.Grow(). - Add extensive unit tests for vector behavior and expand internal + public benchmarks for the new fast paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| marshal.go | Adds vector fast paths, buffer pooling helpers, and generic-path preallocation helper. |
| marshal_vector_test.go | New unit test suite covering round-trip, compatibility, reuse, pool behavior, and prealloc. |
| vector_bench_test.go | Adds pooled/unpooled benchmarks and a simulated write-path benchmark for vectors. |
| tests/bench/bench_vector_public_test.go | Extends public API benchmarks to cover int32/int64 vectors. |
There was a problem hiding this comment.
Pull request overview
This PR adds specialized, non-reflect fast paths for marshaling/unmarshaling common vector<> element types in the GoCQL driver to significantly reduce allocations and improve throughput, along with extensive tests and expanded benchmarks.
Changes:
- Add type-specialized vector marshal/unmarshal implementations for
[]float32,[]float64,[]int32, and[]int64, plus a pooled[]bytebuffer facility for marshal fast paths. - Improve generic vector marshal performance via preallocation (
buf.Grow) when element wire size is known. - Add a comprehensive unit test suite for vector behavior and extend internal + public benchmarks for the new int32/int64 paths and pooled scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
marshal.go |
Adds vector fast paths, buffer pooling helpers, overflow guards, and generic-path preallocation. |
marshal_vector_test.go |
New comprehensive unit tests covering fast paths, edge cases, and pool behavior. |
vector_bench_test.go |
Adds pooled/write-path/round-trip benchmarks and int32/int64 benchmarks. |
tests/bench/bench_vector_public_test.go |
Extends public API benchmarks to cover int32/int64 vectors. |
There was a problem hiding this comment.
Pull request overview
This PR introduces type-specialized fast paths for marshaling/unmarshaling common fixed-width vector element types (float32/float64/int32/int64) to avoid reflect-based per-element work, reducing allocations and significantly improving throughput in the driver’s vector serialization layer.
Changes:
- Added fast-path dispatch in
marshalVector/unmarshalVectorwith dedicated bulk encode/decode implementations for float32/float64/int32/int64 vectors. - Introduced a
sync.Pool-backed byte buffer reuse mechanism for vector marshaling and slice-backing reuse for unmarshaling. - Added extensive unit tests plus expanded internal/public benchmarks for the new fast paths and pooled usage patterns.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| marshal.go | Adds fast-path vector marshal/unmarshal implementations, buffer pooling helpers, and generic-path preallocation. |
| marshal_vector_test.go | Adds a comprehensive unit test suite for vector behavior, pooling, edge cases, and compatibility. |
| vector_bench_test.go | Adds/extends internal benchmarks for pooled marshal, write-path simulation, and int vector types. |
| tests/bench/bench_vector_public_test.go | Extends public API benchmarks to cover int32/int64 vector marshal/unmarshal. |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
March 17, 2026 18:04
9e5efc1 to
234f670
Compare
Author
|
Addressed review feedback: Fixed (this push):
Already fixed in prior revisions:
Not a bug (Copilot false positives):
Not changing (style preference):
|
There was a problem hiding this comment.
Pull request overview
Adds type-specialized marshal/unmarshal fast paths for common vector<...> element types to significantly reduce reflection overhead and allocations in the driver’s value encoding/decoding layer.
Changes:
- Introduces specialized marshal/unmarshal implementations for
[]float32,[]float64,[]int32,[]int64plus async.Pool-backed buffer helper for marshal fast paths. - Adds generic-path preallocation for fixed-size vector element types and improves 0-dimension handling in unmarshal.
- Expands benchmarks and adds a comprehensive new unit test suite for vector behavior/performance characteristics.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
marshal.go |
Adds vector fast paths, pooled buffer helpers, fixed-element-size prealloc, and 0-dimension unmarshal handling. |
marshal_vector_test.go |
New, extensive unit tests for vector marshal/unmarshal correctness, edge cases, and pooling behavior. |
vector_bench_test.go |
Extends internal benchmarks to cover pooled write-path simulations and int32/int64 vectors. |
tests/bench/bench_vector_public_test.go |
Extends public API benchmarks to include int32/int64 vector marshal/unmarshal. |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
March 24, 2026 18:21
b0a2d82 to
118e06c
Compare
mykaul
force-pushed
the
vector-perf-optimize
branch
2 times, most recently
from
April 4, 2026 11:54
432f624 to
9410b42
Compare
Collaborator
|
@mykaul , it is great idea to use pooled buffers, but i think we need to make it generic so that it works for every data type the same way, and don't see any point in targeting vectors specifically. |
mykaul
changed the title
Author
@dkropachev - just because they are large I targeted vectors. I can have a pool per type - or do you prefer one general pool for all types? |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
May 27, 2026 13:45
66ed9aa to
c1b98db
Compare
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 WalkthroughWalkthroughThis PR implements vector marshaling optimizations and buffer pooling for the gocql Cassandra driver. The core change adds reflection-free fast paths in 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
mykaul
force-pushed
the
vector-perf-optimize
branch
from
May 29, 2026 12:21
c1b98db to
72fe318
Compare
Author
Review: vector marshal/unmarshal fast-path correctness (PR #770)Reviewed standalone against FindingsNo HIGH or MEDIUM correctness issues found. The fast paths are well-implemented:
Minor (non-blocking): a generic-path Changes made (test-only)
Test results
Rebase / push
Benchmark recommendation (please run on a quiet machine)The hot path itself was not modified by me, but since this PR is a perf change it should be benchmarked before merge: Compare against Overlap with #744#770 and #744 touch the same vector fast-path territory; #744 is the zero-dimension subset. The nil-vs-empty handling here is consistent with the fix that #744 was addressing — the new differential test pins this behavior so a future change to either PR can't silently re-introduce the #744/#871 divergence. |
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
May 29, 2026
Verified all open Copilot findings on PR scylladb#770 against the current rebased code. The marshal.go fast-path code already addresses the correctness findings; only test improvements were needed. Code findings — verified already correct, no marshal.go change: - A (int overflow dim*4/dim*8): vectorByteSize computes int64(dim)* int64(elemBytes) with an overflow guard, and all marshal/unmarshal sizing + expected-length sites route through it; dim<0 is rejected. - B (error type): vectorByteSize returns a plain error but every caller wraps it via marshalErrorf/unmarshalErrorf, so fast-path errors are proper MarshalError/UnmarshalError. - C (nil-vs-empty): getVectorBuf(0) returns a non-nil make([]byte,0); nil vec -> nil bytes (CQL NULL), non-nil empty -> non-nil 0-len; unmarshal of 0-len non-nil data yields non-nil empty dst. Matches the generic path (covered by the differential test). - E (GoDoc displacement): doc comments are adjacent to their symbols. - F (vectorFixedElemSize switch): the helper is only a buf.Grow capacity hint in the generic path and is guarded by !isLengthType; excluded types are variable-length so unreachable. Doc already documents the deliberate exclusions. - D (array dest validation): unreachable in the fast path — array dests (*[N]T) never match the slice-typed assertions and fall through to the generic reflect path, which already validates array length. Test added. Test changes: - G: replace fragile &slice[:1][0] backing-array-pointer capture with &slice[0] (the slices have len>0 here; [:1][0] panics on len-0). - D: add TestUnmarshalVector_ArrayDestDimensionMismatch covering too-small/too-large/matching arrays and the dim==0 non-zero-array case. - H (-0 sign bit): bit-level assertions via math.Float32bits/Float64bits for negative-zero already present in TestMarshalVector_SpecialValues.
Author
Copilot review feedback — resolution summaryReviewed all open Copilot findings against the current rebased code (branch tip Per-finding verdict
Code vs test changes
Test status
Push decisionPushed ( (No marshal.go change → no re-benchmark required. If reviewers want a refresher anyway: |
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
May 31, 2026
Verified all open Copilot findings on PR scylladb#770 against the current rebased code. The marshal.go fast-path code already addresses the correctness findings; only test improvements were needed. Code findings — verified already correct, no marshal.go change: - A (int overflow dim*4/dim*8): vectorByteSize computes int64(dim)* int64(elemBytes) with an overflow guard, and all marshal/unmarshal sizing + expected-length sites route through it; dim<0 is rejected. - B (error type): vectorByteSize returns a plain error but every caller wraps it via marshalErrorf/unmarshalErrorf, so fast-path errors are proper MarshalError/UnmarshalError. - C (nil-vs-empty): getVectorBuf(0) returns a non-nil make([]byte,0); nil vec -> nil bytes (CQL NULL), non-nil empty -> non-nil 0-len; unmarshal of 0-len non-nil data yields non-nil empty dst. Matches the generic path (covered by the differential test). - E (GoDoc displacement): doc comments are adjacent to their symbols. - F (vectorFixedElemSize switch): the helper is only a buf.Grow capacity hint in the generic path and is guarded by !isLengthType; excluded types are variable-length so unreachable. Doc already documents the deliberate exclusions. - D (array dest validation): unreachable in the fast path — array dests (*[N]T) never match the slice-typed assertions and fall through to the generic reflect path, which already validates array length. Test added. Test changes: - G: replace fragile &slice[:1][0] backing-array-pointer capture with &slice[0] (the slices have len>0 here; [:1][0] panics on len-0). - D: add TestUnmarshalVector_ArrayDestDimensionMismatch covering too-small/too-large/matching arrays and the dim==0 non-zero-array case. - H (-0 sign bit): bit-level assertions via math.Float32bits/Float64bits for negative-zero already present in TestMarshalVector_SpecialValues.
mykaul
force-pushed
the
vector-perf-optimize
branch
from
May 31, 2026 10:54
0d76cca to
82b3a75
Compare
mykaul
marked this pull request as ready for review
June 5, 2026 10:02
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
conn.go (1)
1693-1724:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInstall pooled-buffer cleanup before marshalling loops.
On Line 1693 and Line 1905 paths, cleanup is deferred only after marshalling completes. If marshalling fails mid-loop, already-created vector buffers are not returned to the pool.
♻️ Proposed fix
@@ func (c *Conn) executeQuery(ctx context.Context, qry *Query) (iter *Iter) { - params.values = make([]queryValues, len(values)) - for i := 0; i < len(values); i++ { + cols := info.request.columns + params.values = make([]queryValues, len(values)) + + hasPooledVec := false + for i := 0; i < len(values); i++ { + if vt, ok := cols[i].TypeInfo.(VectorType); ok && vectorBufPoolSubtype(vt) { + hasPooledVec = true + break + } + } + if hasPooledVec { + defer func() { + for i := 0; i < len(params.values) && i < len(cols); i++ { + if vt, ok := cols[i].TypeInfo.(VectorType); ok && vectorBufPoolSubtype(vt) { + putVectorBuf(params.values[i].value) + } + } + }() + } + + for i := 0; i < len(values); i++ { v := ¶ms.values[i] value := values[i] - typ := info.request.columns[i].TypeInfo + typ := cols[i].TypeInfo if err := marshalQueryValue(typ, value, v); err != nil { return &Iter{err: err} } } - - cols := info.request.columns - vals := params.values - hasPooledVec := false - for _, col := range cols { - if vt, ok := col.TypeInfo.(VectorType); ok && vectorBufPoolSubtype(vt) { - hasPooledVec = true - break - } - } - if hasPooledVec { - defer func() { - for i, col := range cols { - if vt, ok := col.TypeInfo.(VectorType); ok && vectorBufPoolSubtype(vt) { - putVectorBuf(vals[i].value) - } - } - }() - } @@ func (c *Conn) executeBatch(ctx context.Context, batch *Batch) (iter *Iter) { var vectorBufs [][]byte + defer func() { + for _, buf := range vectorBufs { + putVectorBuf(buf) + } + }() @@ - if len(vectorBufs) > 0 { - defer func() { - for _, buf := range vectorBufs { - putVectorBuf(buf) - } - }() - }Also applies to: 1905-1971
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@conn.go` around lines 1693 - 1724, The pooled-vector cleanup defer must be installed before the marshalling loop so partially-marshalled vector buffers are returned on error: detect whether any column TypeInfo is a poolable VectorType (use info.request.columns, VectorType and vectorBufPoolSubtype) immediately after allocating params.values, then, if true, install a defer that iterates over captured cols/vals and calls putVectorBuf(vals[i].value); only after that call marshalQueryValue for each value (the loop that currently returns &Iter{err: err} on failure) so any buffers allocated prior to an error are released; ensure the defer captures the same params.values slice used by the marshalling loop to avoid missing entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@conn.go`:
- Around line 1693-1724: The pooled-vector cleanup defer must be installed
before the marshalling loop so partially-marshalled vector buffers are returned
on error: detect whether any column TypeInfo is a poolable VectorType (use
info.request.columns, VectorType and vectorBufPoolSubtype) immediately after
allocating params.values, then, if true, install a defer that iterates over
captured cols/vals and calls putVectorBuf(vals[i].value); only after that call
marshalQueryValue for each value (the loop that currently returns &Iter{err:
err} on failure) so any buffers allocated prior to an error are released; ensure
the defer captures the same params.values slice used by the marshalling loop to
avoid missing entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4af3cac7-ef8e-4e7a-899e-fb458cbb3084
📒 Files selected for processing (7)
conn.gohelpers_bench_test.gomarshal.gomarshal_test.gomarshal_vector_test.gotests/bench/bench_vector_public_test.govector_bench_test.go
mykaul
marked this pull request as draft
June 5, 2026 10:15
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
Jun 5, 2026
Verified all open Copilot findings on PR scylladb#770 against the current rebased code. The marshal.go fast-path code already addresses the correctness findings; only test improvements were needed. Code findings — verified already correct, no marshal.go change: - A (int overflow dim*4/dim*8): vectorByteSize computes int64(dim)* int64(elemBytes) with an overflow guard, and all marshal/unmarshal sizing + expected-length sites route through it; dim<0 is rejected. - B (error type): vectorByteSize returns a plain error but every caller wraps it via marshalErrorf/unmarshalErrorf, so fast-path errors are proper MarshalError/UnmarshalError. - C (nil-vs-empty): getVectorBuf(0) returns a non-nil make([]byte,0); nil vec -> nil bytes (CQL NULL), non-nil empty -> non-nil 0-len; unmarshal of 0-len non-nil data yields non-nil empty dst. Matches the generic path (covered by the differential test). - E (GoDoc displacement): doc comments are adjacent to their symbols. - F (vectorFixedElemSize switch): the helper is only a buf.Grow capacity hint in the generic path and is guarded by !isLengthType; excluded types are variable-length so unreachable. Doc already documents the deliberate exclusions. - D (array dest validation): unreachable in the fast path — array dests (*[N]T) never match the slice-typed assertions and fall through to the generic reflect path, which already validates array length. Test added. Test changes: - G: replace fragile &slice[:1][0] backing-array-pointer capture with &slice[0] (the slices have len>0 here; [:1][0] panics on len-0). - D: add TestUnmarshalVector_ArrayDestDimensionMismatch covering too-small/too-large/matching arrays and the dim==0 non-zero-array case. - H (-0 sign bit): bit-level assertions via math.Float32bits/Float64bits for negative-zero already present in TestMarshalVector_SpecialValues.
mykaul
force-pushed
the
vector-perf-optimize
branch
from
June 5, 2026 11:11
82b3a75 to
addfc51
Compare
Author
Update — addressed CodeRabbit finding🟡 Minor (outside-diff) — pooled vector buffers leak on mid-loop marshal failure ( In both
Timing is unchanged for the success path (defer still runs at function exit, after
|
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
Jun 7, 2026
…-reuse tests
float32 and int32 subtests already had an explicit length check before
taking the backing-array pointer (&result[0]). The float64 and int64
subtests were missing those guards, so a hypothetical bug in
unmarshalVectorFloat64/Int64 that returned nil error without populating
the destination would silently panic instead of producing a diagnostic
failure.
Add the same 'if len(result) != dim { t.Fatalf(...) }' checks for
consistency and defensive test quality (addresses Copilot round-4
finding on PR scylladb#770).
Author
|
@copilot-pull-request-reviewer Thanks for the review across 6 rounds. Here is how every finding was addressed, all in current HEAD (4812592): Panic - &result[:1][0] on zero-length slices (IDs 2930969041/68/90/108, 2968086667/692): Fixed in addfc51 - replaced with &result[0]. Commit 4812592 adds the missing len checks in float64 and int64 subtests too. dim*N integer overflow (IDs 2936438432/41/49/56/61/64/67/75): All sizing routes through vectorByteSize(dim, N) using int64 arithmetic with overflow guard. BCE hints are only reached after that check and dim>0 guard. vectorByteSize error type (ID 2937025220): Every call site wraps via marshalErrorf / unmarshalErrorf. getVectorBuf(0) returning nil (IDs 2937025229, 2942089314): Fixed in 62a7f4a - returns make([]byte, 0) for size==0. dim==0 fast paths leaving *dst nil (IDs 2937025239/45/54/61): Fixed in 62a7f4a - all fast-path unmarshal functions handle dim==0 explicitly. Generic marshal returns nil for dim==0 (ID 2968086682): Fixed in 62a7f4a - early 'if n == 0 { return make([]byte, 0), nil }' guard added. Empty vector tests pass for nil (ID 2942089284): Already checked - TestMarshalVector_EmptyVector asserts data != nil explicitly. dim==0 array destination validation (ID 2942089331): Fast paths only match *[]T; array destinations fall through to generic path which validates array length. TestUnmarshalVector_ArrayDestDimensionMismatch covers this. Generic unmarshal divide-by-zero (ID 2930752007): Guarded by 'if info.Dimensions == 0' at line 971, before the division. reflect.DeepEqual / -0.0 (IDs 2930752034/52): TestMarshalVector_SpecialValues already has explicit bit-level math.Float32bits/Float64bits checks. Doc comment placement (IDs 2930751977, 2930969009) and vectorFixedElemSize comment (ID 2936438458): All accurate in current code, no change needed. |
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
Aug 19, 2026
Verified all open Copilot findings on PR scylladb#770 against the current rebased code. The marshal.go fast-path code already addresses the correctness findings; only test improvements were needed. Code findings — verified already correct, no marshal.go change: - A (int overflow dim*4/dim*8): vectorByteSize computes int64(dim)* int64(elemBytes) with an overflow guard, and all marshal/unmarshal sizing + expected-length sites route through it; dim<0 is rejected. - B (error type): vectorByteSize returns a plain error but every caller wraps it via marshalErrorf/unmarshalErrorf, so fast-path errors are proper MarshalError/UnmarshalError. - C (nil-vs-empty): getVectorBuf(0) returns a non-nil make([]byte,0); nil vec -> nil bytes (CQL NULL), non-nil empty -> non-nil 0-len; unmarshal of 0-len non-nil data yields non-nil empty dst. Matches the generic path (covered by the differential test). - E (GoDoc displacement): doc comments are adjacent to their symbols. - F (vectorFixedElemSize switch): the helper is only a buf.Grow capacity hint in the generic path and is guarded by !isLengthType; excluded types are variable-length so unreachable. Doc already documents the deliberate exclusions. - D (array dest validation): unreachable in the fast path — array dests (*[N]T) never match the slice-typed assertions and fall through to the generic reflect path, which already validates array length. Test added. Test changes: - G: replace fragile &slice[:1][0] backing-array-pointer capture with &slice[0] (the slices have len>0 here; [:1][0] panics on len-0). - D: add TestUnmarshalVector_ArrayDestDimensionMismatch covering too-small/too-large/matching arrays and the dim==0 non-zero-array case. - H (-0 sign bit): bit-level assertions via math.Float32bits/Float64bits for negative-zero already present in TestMarshalVector_SpecialValues.
mykaul
added a commit
to mykaul/gocql
that referenced
this pull request
Aug 19, 2026
…-reuse tests
float32 and int32 subtests already had an explicit length check before
taking the backing-array pointer (&result[0]). The float64 and int64
subtests were missing those guards, so a hypothetical bug in
unmarshalVectorFloat64/Int64 that returned nil error without populating
the destination would silently panic instead of producing a diagnostic
failure.
Add the same 'if len(result) != dim { t.Fatalf(...) }' checks for
consistency and defensive test quality (addresses Copilot round-4
finding on PR scylladb#770).
mykaul
force-pushed
the
vector-perf-optimize
branch
from
August 19, 2026 17:40
4812592 to
ec02e89
Compare
Add type-specialized fast paths for vector<float>, vector<double>,
vector<int>, and vector<bigint> that bypass reflect-based per-element
marshaling in favor of direct encoding/binary bulk conversion.
Changes in marshal.go:
- Type switches in marshalVector()/unmarshalVector() dispatch to
dedicated functions for []float32, []float64, []int32, []int64
before falling through to the generic reflect path.
- 8 new functions: marshalVectorFloat32, marshalVectorFloat64,
unmarshalVectorFloat32, unmarshalVectorFloat64, marshalVectorInt32,
marshalVectorInt64, unmarshalVectorInt32, unmarshalVectorInt64.
- sync.Pool buffer reuse (vectorBufPool/getVectorBuf/putVectorBuf)
for zero-alloc steady state when callers return buffers after
the framer copies them. 64KiB cap prevents pool bloat.
- Unmarshal fast paths reuse destination slice backing array when
capacity is sufficient (zero-alloc steady state on read path).
- Generic path preallocation via vectorFixedElemSize() + buf.Grow()
for non-fast-path fixed-size types (e.g. UUID, timestamp).
- vectorByteSize() helper guards against integer overflow on 32-bit
platforms with corrupt or adversarial schema metadata.
- All fast-path errors are wrapped as MarshalError/UnmarshalError
for consistent error typing.
- dim=0 vectors correctly encode as non-nil empty values (not CQL NULL)
in both fast paths and generic path.
- Negative dimensions are rejected with clear error messages.
Benchmark results for vector<float, 1536> (typical embedding dimension):
Marshal (baseline -> optimized):
86.4 us/op -> 3.4 us/op (25x faster)
3081 allocs -> 2 allocs (99.94% fewer)
28632 B/op -> 6172 B/op (78% less memory)
Marshal with pool return (steady state):
86.4 us/op -> 1.6 us/op (54x faster)
3081 allocs -> 2 allocs (99.94% fewer)
28632 B/op -> 48 B/op (99.8% less memory)
Unmarshal (baseline -> optimized):
60.2 us/op -> 1.5 us/op (41x faster)
2 allocs -> 0 allocs (100% fewer)
6168 B/op -> 0 B/op (100% less memory)
Round-trip (baseline -> optimized, pooled):
147.8 us/op -> 3.1 us/op (48x faster)
3083 allocs -> 2 allocs (99.94% fewer)
34800 B/op -> 48 B/op (99.9% less memory)
Throughput: 80 MB/s -> 3.5 GB/s (geomean, +2900%)
New test files:
- marshal_vector_test.go: 58+ unit subtests across 13 categories
(round-trip, byte-compat, slice-reuse, nil, dimension-mismatch,
empty-vector, pointer-to-slice, special-values, pool-concurrency,
oversized-not-pooled, fixed-elem-size, generic-prealloc).
- vector_bench_test.go: extended with int32/int64 and pooled benchmarks.
- tests/bench/bench_vector_public_test.go: public API benchmarks for
int32/int64 marshal/unmarshal.
Subsumes PR scylladb#744 (float fast paths) and PR scylladb#745 (generic prealloc).
Extends with int32/int64 fast paths and buffer pooling not covered by
any existing PR.
Return pooled vector buffers to vectorBufPool after the framer copies marshalled bytes in executeQuery and executeBatch. This completes the zero-alloc steady-state cycle for vector marshal operations. In executeQuery, a defer after the marshal loop returns buffers for columns identified as pooled vector types (float32, float64, int32, int64). In executeBatch, vector buffers are collected across all batch statements and returned via a single defer. The vectorBufPoolSubtype helper centralizes the type check to keep the two call sites consistent with the marshal fast paths. Includes unit tests covering vectorBufPoolSubtype classification, single-query and batch pool return simulation, and non-pooled type safety.
Add dedicated marshal/unmarshal fast paths for UUID and TimeUUID vector elements, following the same pattern as the existing float32/float64/ int32/int64 fast paths. UUID is [16]byte with no endian conversion needed, so the fast path uses a simple copy() loop. Uses pooled buffers via getVectorBuf for zero-alloc steady state on the marshal path, and reuses the destination slice backing array on the unmarshal path. Benchmarks (vs generic reflection path): - Marshal: ~90% faster (10x speedup), 99%+ fewer allocations - Unmarshal: ~97% faster (30-35x speedup), zero allocations - Marshal+pool: additional 4x over non-pooled marshal
…arse VectorType embeds NativeType but had no NewWithError() method, so calls fell through to NativeType.NewWithError() which hit the TypeCustom fallback: goType() → asVectorType() → re-parse the full Java type string (e.g. 'org.apache.cassandra.db.marshal.VectorType(FloatType, 1536)') on every invocation. This is called per-column per-row by RowData() and MapScan, making it a hot path for vector workloads. Add VectorType.NewWithError() with fast paths for all common element types (float32, float64, int32, int64, UUID, string, bool, etc.) that return *[]T directly without reflection or string parsing. Fallback for exotic subtypes still uses SubType.NewWithError() + reflect.SliceOf but avoids the asVectorType() re-parse. Also fix zero-dimension error messages in fast-path unmarshal functions to be consistent with the generic path (check dim==0 before byte-size validation), fix copyright header in marshal_vector_test.go, and fix pre-existing session_unit_test.go build error from origin/master (hostId string → UUID type mismatch). Benchmark results (VectorType.NewWithError vs NativeType fallback): VectorType: ~17 ns/op, 24 B/op, 1 allocs/op NativeType_fallback: ~170 ns/op, 92 B/op, 4 allocs/op → 10x faster, 75% fewer allocations
…or vectors Adds TestMarshalVector_FastPathMatchesGeneric which forces the generic reflect-based marshal path (via named slice types that fail the fast-path type assertions) and compares its output byte-for-byte against the optimized fast path for float32/float64/int32/int64/UUID vectors across nil, empty, single, many, NaN/Inf/negative-zero, and min/max-value cases. Specifically guards the nil-vs-empty distinction (nil -> nil bytes / CQL null; empty -> non-nil zero-length) which was the HIGH-severity bug class in scylladb#744/scylladb#871. Test-only change.
Verified all open Copilot findings on PR scylladb#770 against the current rebased code. The marshal.go fast-path code already addresses the correctness findings; only test improvements were needed. Code findings — verified already correct, no marshal.go change: - A (int overflow dim*4/dim*8): vectorByteSize computes int64(dim)* int64(elemBytes) with an overflow guard, and all marshal/unmarshal sizing + expected-length sites route through it; dim<0 is rejected. - B (error type): vectorByteSize returns a plain error but every caller wraps it via marshalErrorf/unmarshalErrorf, so fast-path errors are proper MarshalError/UnmarshalError. - C (nil-vs-empty): getVectorBuf(0) returns a non-nil make([]byte,0); nil vec -> nil bytes (CQL NULL), non-nil empty -> non-nil 0-len; unmarshal of 0-len non-nil data yields non-nil empty dst. Matches the generic path (covered by the differential test). - E (GoDoc displacement): doc comments are adjacent to their symbols. - F (vectorFixedElemSize switch): the helper is only a buf.Grow capacity hint in the generic path and is guarded by !isLengthType; excluded types are variable-length so unreachable. Doc already documents the deliberate exclusions. - D (array dest validation): unreachable in the fast path — array dests (*[N]T) never match the slice-typed assertions and fall through to the generic reflect path, which already validates array length. Test added. Test changes: - G: replace fragile &slice[:1][0] backing-array-pointer capture with &slice[0] (the slices have len>0 here; [:1][0] panics on len-0). - D: add TestUnmarshalVector_ArrayDestDimensionMismatch covering too-small/too-large/matching arrays and the dim==0 non-zero-array case. - H (-0 sign bit): bit-level assertions via math.Float32bits/Float64bits for negative-zero already present in TestMarshalVector_SpecialValues.
…-reuse tests
float32 and int32 subtests already had an explicit length check before
taking the backing-array pointer (&result[0]). The float64 and int64
subtests were missing those guards, so a hypothetical bug in
unmarshalVectorFloat64/Int64 that returned nil error without populating
the destination would silently panic instead of producing a diagnostic
failure.
Add the same 'if len(result) != dim { t.Fatalf(...) }' checks for
consistency and defensive test quality (addresses Copilot round-4
finding on PR scylladb#770).
mykaul
force-pushed
the
vector-perf-optimize
branch
from
August 24, 2026 09:41
ec02e89 to
260ac41
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Type-specialized fast paths for
vector<float>,vector<double>,vector<int>,vector<bigint>, andvector<uuid>/vector<timeuuid>that bypass reflect-based per-element marshaling in favor of directencoding/binarybulk conversion, plussync.Poolbuffer reuse wired into the connection write path, and aVectorType.NewWithError()fast path that eliminates the expensivegoType()→asVectorType()re-parse on every call.Commit 1:
d527db1perf: optimize vector marshal/unmarshal for float32/float64/int32/int64Fast-path type switches, 8 dedicated marshal/unmarshal functions,
sync.Poolinfrastructure (getVectorBuf/putVectorBuf), unmarshal slice reuse, generic-pathbuf.Grow()preallocation viavectorFixedElemSize(), and comprehensive tests (58 subtests across 13 categories).Commit 2:
04f0783perf: wire putVectorBuf into connection write pathAdds
defer putVectorBuf(...)calls inexecuteQuery()andexecuteBatch()inconn.go, so production callers return pooled marshal buffers after the framer copies them. This closes the pool lifecycle and achieves 48 B/op steady-state on the write path.Commit 3:
d48f44fperf: add UUID/TimeUUID vector fast pathAdds marshal/unmarshal fast paths for
vector<uuid>andvector<timeuuid>— bulkcopy()of fixed 16-byte elements with zero per-element allocations. UUID vectors are common in similarity search use cases (storing document IDs alongside embeddings).Commit 4:
66ed9aaperf: add VectorType.NewWithError() to avoid goType/asVectorType re-parseVectorTypeembedsNativeTypebut had noNewWithError()method. When called, it dispatched toNativeType.NewWithError()which hitTypeCustom→goType()→asVectorType(), re-parsing the full Java type string on every call. The new method returns*[]SubTypedirectly: 10.7x faster (181ns → 17ns), 75% fewer allocs (4 → 1), 74% less memory (92B → 24B).Headline numbers (
vector<float, 1536>, typical embedding dimension)NewWithError()forVectorType(181ns → 17ns)Benchmark results
All benchmarks: 6 iterations,
benchstat, all p=0.002. Machine: 12th Gen Intel Core i7-1270P.Master =
3881f1e(origin/master), Optimized =66ed9aa(this branch HEAD).Latency (ns/op) — Master vs Optimized (all 4 commits)
Pool wiring benefit (Commit 2) — production write path
The table above measures
Marshal()/Unmarshal()via the public API, which does not return buffers to the pool. In production,executeQuery()/executeBatch()return the buffer viaputVectorBuf()after the framer copies it. ThePooledbenchmarks simulate this:Memory with pool return is 48 B/op constant, regardless of vector dimension or element type (from
sync.Poolinterface boxing overhead, irreducible). Compare to master: 18,456 B/op for float32/dim_1536, 98,328 B/op for UUID/dim_1536.Full benchstat details: memory and allocations (click to expand)
Memory (B/op)
Allocations (allocs/op)
Pooled marshal memory (B/op) — with pool return
What changed
marshal.goFast-path type switches in
marshalVector()andunmarshalVector()— before the existing reflect-based generic path, a switch oninfo.SubType.Type()intercepts[]float32,[]float64,[]int32,[]int64,[]UUIDand dispatches to 10 dedicated functions. Falls through to the generic path for all other types.10 new marshal/unmarshal functions —
marshalVectorFloat32/Float64/Int32/Int64/UUIDand correspondingunmarshalVector*. Float/int functions useencoding/binary.BigEndian.PutUint32/PutUint64withmath.Float32bits/Float64bits. UUID functions use bulkcopy()of 16-byte elements.sync.Poolbuffer reuse —vectorBufPool,getVectorBuf(size),putVectorBuf(buf)with 64 KiB cap guard.Unmarshal slice reuse — All unmarshal fast paths reuse the destination slice's backing array when capacity is sufficient, achieving zero allocations on repeated reads.
Generic path preallocation —
vectorFixedElemSize()returns the wire-format byte size for fixed-length CQL types. The generic path callsbuf.Grow()upfront.VectorType.NewWithError()— Returns*[]SubTypedirectly without going throughgoType()→asVectorType(). 10.7x faster, 75% fewer allocs.conn.goexecuteQuery()andexecuteBatch()calldefer putVectorBuf(...)on eachqueryValues.value, closing the pool lifecycle.Test files
marshal_vector_test.go— 58 unit subtests across 13 categories including UUID-specific testsvector_bench_test.go— Benchmarks for all 5 types: marshal, pooled marshal, unmarshal, across dimensions 128/384/768/1536marshal_test.go—TestVectorNewWithErrorConsistentWithGoType,TestVectorNewWithErrorReturnsSlicePointerhelpers_bench_test.go—BenchmarkVectorNewWithError,BenchmarkRowDataWithVectorHow the bottleneck was eliminated
The original generic path for
vector<float, 1536>:Marshal()1,536 times through reflect dispatch[]byteviaencFloat32(1,536 allocs)bytes.Bufferthat grew incrementally (additional allocs)framer.bufbywriteBytes()The fast path:
getVectorBuf(6144)(pooled, zero-alloc steady state)binary.BigEndian.PutUint32(buf[i*4:], math.Float32bits(v))putVectorBuf()returns the buffer to the pool afterc.exec()Design decisions
framer.bufwould save ~0.4 µs (~21% marginal over the pooled path) but requires invasive changes to thequeryValuesstruct used by all query paths. The risk/reward ratio is unfavorable.isVectorVariableLengthType) was deliberately skipped — there is a discrepancy in how some types are handled between Cassandra and ScyllaDB implementations. We focus only on the 5 types where the wire format is unambiguous.Relationship to open PRs
Replaces PR #744 (float fast paths) and PR #745 (generic prealloc)
This PR is a strict superset of both:
sync.Poolbuffer reuseconn.goVectorType.NewWithError()vectorFixedElemSize()helperbuf.Grow()preallocIf this PR merges first, #744 and #745 become no-ops and should be closed.
Orthogonal to PRs #751, #752, #753
These PRs reduce per-request allocations in the connection/framing layer. This PR reduces allocations in the marshal/unmarshal layer. They are fully complementary — different files, different allocation sites, additive benefits.
Note: PR #749 (pool write-side framers) was closed — fully superseded by
3e1e7e4on master.Depends on PR #838
PR #838 fixes a pre-existing build failure in
session_unit_test.gowherehostIdchanged fromstringtoUUIDbut test literals were not updated. This branch carries the same fix; once #838 merges, the fix becomes a no-op on rebase.