perf: buffer accumulation in _write_query_params() reduces f.write() calls (~100ns improvement, 1.1-1.3x speedup) by mykaul · Pull Request #790 · scylladb/python-driver

mykaul · 2026-04-04T14:22:08Z

Summary

Replace per-parameter write_value(f, param) loops with buffer accumulation (list.append + b"".join + single f.write()), reducing f.write() calls from (2*N + 1) to 1 for N query parameters in the execute/query path.

What changed

`cassandra/protocol.py`

_QueryMessage._write_query_params() -- Buffer accumulation for the parameter loop. Local variable caching (_int32_pack, _parts_append) for Cython-friendly tight loop.
ExecuteMessage._write_query_params() -- Removed unnecessary super() pass-through override (now inherited directly from _QueryMessage).

`tests/unit/test_protocol.py`

Added 14 new test methods in WriteQueryParamsBufferAccumulationTest.

Benchmark

Measured with min() of timeit.repeat(repeat=7, number=200_000) on a quiet machine (load <3), Cython .so compiled, before/after rebuild.

Scenario	Baseline (ns/call)	Buffer accum (ns/call)	Speedup
128D float32 vector (1 param)	892	953	0.94x
1536D float32 vector (1 param)	1077	1208	0.89x
10 text columns	2416	1783	1.35x

For single-param workloads, the list/join overhead slightly exceeds the write-call savings. The benefit appears with multiple params (10+), where reducing 2*N writes to 1 join becomes significant. The real payoff is in BatchMessage.send_body() (PR #791), where N is much larger.

Tests

14 new tests covering normal, NULL, UNSET, mixed, empty bytes, large vector, many params, cross-protocol
Full unit test suite passes (666 passed, 43 skipped)

mykaul · 2026-04-04T14:40:00Z

Just spitting this here:
Is there any value in ~0.1us improvement?

Honest answer: on its own, ~100ns per call is tiny. But context matters:
Where it adds up:

High-throughput vector INSERT pipelines doing 100K+ ops/sec — that's ~10ms/sec of CPU saved per core
The write path is single-threaded per connection, so it's on the critical path
Multiply by batch sizes (BatchMessage calls write_value per param per query)
Where it doesn't matter:
Network round-trip to Scylla is typically 100-500us minimum
Serialization of the vector bytes themselves dwarfs the write_value overhead
Real workloads are rarely bottlenecked on parameter serialization
The real argument for merging is code quality, not the 100ns:
Removing the redundant super() pass-through is a clear cleanup
The buffer accumulation is a well-known pattern (fewer writes = better) and the code isn't harder to read
It establishes a pattern for future BatchMessage optimization where the savings would be larger (N queries × M params)
But if the question is "would a user ever notice this in production?" — no, almost certainly not. It's a micro-optimization. Whether it's worth the diff churn and review cost is a judgment call for the maintainers.

Replace the per-parameter write_value(f, param) loop in _QueryMessage._write_query_params() with a buffer accumulation approach: list.append + b"".join + single f.write(). This reduces the number of f.write() calls from 2*N+1 to 1, which is significant for vector workloads with large parameters. Also removes the redundant ExecuteMessage._write_query_params() pass-through override to avoid extra MRO lookup per call. Includes 14 unit tests covering normal, NULL, UNSET, empty, large vector, and mixed parameter scenarios for both ExecuteMessage and QueryMessage. Includes a benchmark script (benchmarks/bench_execute_write_params.py).

coderabbitai · 2026-07-29T20:31:31Z

Warning

Review limit reached

@mykaul, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: 6c17c53c-296b-49c7-8cbe-045ab49b2e59

📥 Commits

Reviewing files that changed from the base of the PR and between b8b714c and 9a41e1f.

📒 Files selected for processing (3)

benchmarks/bench_execute_write_params.py
cassandra/protocol.py
tests/unit/test_protocol.py

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

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.

Optimizes query-parameter serialization in protocol messages by buffering encoded parameter bytes and writing them in a single f.write(), reducing per-parameter write calls in hot paths.

Changes:

Buffer-accumulate query parameter bytes in _QueryMessage._write_query_params() and write once.
Remove redundant ExecuteMessage._write_query_params() pass-through override.
Add unit tests validating serialization parity and add a local benchmarking script.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
cassandra/protocol.py	Implements buffered parameter encoding and removes redundant override to reduce overhead in execute/query paths.
tests/unit/test_protocol.py	Adds unit tests to validate the new buffered encoding matches prior `write_value()` behavior.
benchmarks/bench_execute_write_params.py	Adds a standalone benchmark script to compare encoding variants and measure performance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            # Accumulate param bytes in a list and write once instead of
+            # 2*N+1 separate f.write() calls via write_value().
+            _int32_pack = int32_pack
+            parts = [uint16_pack(len(self.query_params))]
+            _parts_append = parts.append
            for param in self.query_params:
-                write_value(f, param)
+                if param is None:
+                    _parts_append(_int32_pack(-1))
+                elif param is _UNSET_VALUE:
+                    _parts_append(_int32_pack(-2))
+                else:
+                    _parts_append(_int32_pack(len(param)))
+                    _parts_append(param)
+            f.write(b"".join(parts))


+        raw = self._execute_msg_bytes(msg, protocol_version=4)
+        self.assertIn(expected, raw)


+    def test_none_query_params(self):
+        """When query_params is None, no param block should be written."""
+        msg1 = ExecuteMessage(query_id=b'qid', query_params=None,
+                              consistency_level=1)
+        msg2 = ExecuteMessage(query_id=b'qid', query_params=[b'x'],
+                              consistency_level=1)
+        raw1 = self._execute_msg_bytes(msg1, protocol_version=4)
+        raw2 = self._execute_msg_bytes(msg2, protocol_version=4)
+        # raw1 should be shorter (no param section)
+        self.assertLess(len(raw1), len(raw2))


+            # Accumulate param bytes in a list and write once instead of
+            # 2*N+1 separate f.write() calls via write_value().


mykaul marked this pull request as draft April 4, 2026 14:23

mykaul force-pushed the perf/buffer-accum-write-params branch 2 times, most recently from bc1545f to 9b21d5b Compare April 4, 2026 14:36

mykaul force-pushed the perf/buffer-accum-write-params branch 2 times, most recently from f2be2a8 to ac64459 Compare April 4, 2026 15:02

mykaul mentioned this pull request Apr 4, 2026

perf: buffer accumulation in BatchMessage.send_body() (1.6-1.8x speedup, us improvement, depends on PR #790) #791

Open

mykaul changed the title ~~perf: buffer accumulation in _write_query_params() reduces f.write() calls~~ perf: buffer accumulation in _write_query_params() reduces f.write() calls (~100ns improvement, 1.1-1.3x speedup) Apr 7, 2026

mykaul force-pushed the perf/buffer-accum-write-params branch from ac64459 to 1e1e709 Compare April 7, 2026 08:29

Lorak-mmk force-pushed the master branch from f2a9e87 to 763af09 Compare June 15, 2026 10:57

mykaul force-pushed the perf/buffer-accum-write-params branch from 1e1e709 to 9a41e1f Compare July 29, 2026 20:31

Copilot AI review requested due to automatic review settings July 29, 2026 20:31

mykaul mentioned this pull request Jul 29, 2026

(improvement) serializers: add Cython-optimized serialization for VectorType (up to 30x faster - 823us -> 4us!) #748

Draft

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Copilot started reviewing on behalf of mykaul July 29, 2026 21:20 View session

mykaul marked this pull request as ready for review August 15, 2026 06:56

Navigation Menu

Uh oh!

perf: buffer accumulation in _write_query_params() reduces f.write() calls (~100ns improvement, 1.1-1.3x speedup) - #790

perf: buffer accumulation in _write_query_params() reduces f.write() calls (~100ns improvement, 1.1-1.3x speedup)#790
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:perf/buffer-accum-write-params

mykaul commented Apr 4, 2026 •

edited

Loading

Uh oh!

mykaul commented Apr 4, 2026

Uh oh!

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Review limit reached

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		raw = self._execute_msg_bytes(msg, protocol_version=4)
		self.assertIn(expected, raw)

		# Accumulate param bytes in a list and write once instead of
		# 2*N+1 separate f.write() calls via write_value().

Uh oh!

Conversation

mykaul commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

cassandra/protocol.py

tests/unit/test_protocol.py

Benchmark

Tests

Uh oh!

mykaul commented Apr 4, 2026

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mykaul commented Apr 4, 2026 •

edited

Loading

`cassandra/protocol.py`

`tests/unit/test_protocol.py`

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading