(improvement) perf: remove copies on the read path (us improvements - x1.5-3.7 speedup!) - #734
(improvement) perf: remove copies on the read path (us improvements - x1.5-3.7 speedup!)#734mykaul wants to merge 5 commits into
Conversation
|
#630 would have helped with wide_5k_doubles of course. |
mykaul
changed the title
3e11f41 to
213c0e8
Compare
mykaul
changed the title
mykaul
changed the title
|
Important 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 |
|
Rebased onto current Since this PR's whole point is removing copies on the hot read path, I went through every copy elimination looking specifically for use-after-free/aliasing risk (a stale reference outliving the buffer it points into, or a reference to something that later gets mutated in place):
One real gap I did close (amended into CI: all required checks (Integration tests on libev/asyncio/asyncore × Python 3.11–3.14t, Docs build, security/snyk) are green on the latest push. "Test wheels building" shows red, but it's a Ran the full No open review threads to resolve. Still a draft as requested. |
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 reduces memory copying along the driver read/decode path to improve latency and throughput, especially for large payloads, by replacing io.BytesIO-based reads with lighter and more zero-copy-friendly primitives.
Changes:
- Introduces
BytesReaderand wires it intoProtocolHandler.decode_message()to avoidio.BytesIO(body)copying. - Adds zero-copy handoff support to the Cython row parser via
BytesIOReader(..., offset)andremaining_buffer(). - Optimizes connection buffering/reset and adds unit tests plus an isolated decode benchmark script.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
cassandra/protocol.py |
Adds BytesReader and switches decode to use it instead of io.BytesIO. |
cassandra/row_parser.pyx |
Avoids f.read() copy by using (buffer, offset) handoff into BytesIOReader. |
cassandra/bytesio.pyx / cassandra/bytesio.pxd |
Adds offset support to BytesIOReader for suffix reads without copying. |
cassandra/connection.py |
Reduces copies by using getbuffer(), adds _reset_buffer, and bumps recv buffer size. |
tests/unit/test_protocol.py |
Adds unit tests for BytesReader and end-to-end decode_message coverage. |
tests/unit/test_connection.py |
Adds tests for _ConnectionIOBuffer._reset_buffer. |
tests/unit/test_bytesio_reader.py |
Adds Cython-only tests for BytesIOReader offset construction/validation. |
benchmarks/decode_benchmark.py |
Adds a standalone benchmark to measure decode performance across scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace getvalue() with getbuffer() memoryview in _read_frame_header and frame body extraction to avoid full-buffer copies. Add _reset_buffer() helper using getbuffer()[pos:] instead of read() to reduce allocations. Wrap memoryview usage in try/finally to ensure release before mutation. Increase in_buffer_size from 4096 to 16384 to reduce recv() call overhead.
Introduce a lightweight BytesReader class that operates directly on bytes data without BytesIO overhead. Materializes memoryview to bytes once in __init__ instead of checking on every read(). Includes remaining_buffer() method for zero-copy handoff to Cython parsers.
Add offset parameter to BytesIOReader so it can start reading from the middle of an existing buffer, avoiding the full-body copy at the Python-to-Cython boundary. Update row_parser.pyx to use f.remaining_buffer() for zero-copy handoff with hasattr fallback. Track _initial_offset for error recovery.
13 BytesReader tests covering read operations, remaining_buffer(), memoryview materialization, empty data, and EOFError handling. 9 BytesIOReader tests covering offset initialization, boundary conditions, read behavior with offset, and error cases.
…pact Standalone benchmark (no cluster required) that constructs synthetic RESULT/ROWS wire-format bodies and measures ProtocolHandler.decode_message() throughput across 8 scenarios (small to 16MB, narrow to 20-column wide). Pins to a single CPU core via sched_setaffinity for consistent results. Supports both Cython and pure-Python paths, with --cprofile option.
This patch set aims to reduce the memory copies we perform in the read path, to improve overall performance - mainly reduce latency on the processing side of the driver receiving the payload.
This is more effective on larger payloads of course.
Comparison to master (Cython path, 10 iterations, CPU pinned, all times in microseconds):
(note - we can see that wide_5k_doubles is bottlenecked on something else - CPU processing of this payload - unpacking it. This may be optimized in a different PR)
./docs/source/.Fixes:annotations to PR description.