Skip to content

perf: inline row decoding and eliminate closures in recv_results_rows (100's to 1000's of ns, x1.3-1.8 speedup, Python only) - #765

Draft
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:perf/inline-row-decode
Draft

perf: inline row decoding and eliminate closures in recv_results_rows (100's to 1000's of ns, x1.3-1.8 speedup, Python only)#765
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:perf/inline-row-decode

Conversation

@mykaul

@mykaulmykaul commented Mar 25, 2026

Copy link
Copy Markdown

Summary

  • Split recv_results_rows into fast path (no column encryption) and slow path (CE enabled)
  • Eliminate per-call closure allocation and merge two-pass row processing into single-pass decoding

Note: This optimization applies to the pure Python decode path only. When Cython extensions are compiled (the default for pip-installed packages), FastResultMessage from row_parser.pyx replaces recv_results_rows entirely. Users running without Cython (e.g., environments where C compilation is unavailable, or explicit use of _ProtocolHandler) will benefit from this change.

Details

Problem

The current recv_results_rows has three sources of overhead on every call:

  1. Two passes over row data: First recv_row reads all raw bytes into a list[list[bytes]], then decode_row iterates again to deserialize — doubling iteration and creating intermediate lists that are immediately discarded.

  2. Per-call closures: decode_val and decode_row are defined as closures inside recv_results_rows, meaning Python allocates new function objects on every result set.

  3. Unconditional ColDesc creation: ColDesc namedtuples are built for every column even when column encryption is not configured (the vast majority of deployments).

Solution

Fast path (no column encryption — the common case):

  • _decode_row_inline(f, colcount, col_types, protocol_version) reads each column's size, reads the bytes, and immediately calls from_binary() — one pass, no intermediate list
  • ColDesc creation is skipped entirely
  • No closures allocated

Slow path (column encryption enabled):

  • Preserves the existing two-pass logic (needed because CE must decrypt before type decoding)
  • decode_val/decode_row moved to module-level functions (_decode_val_ce, _decode_row_ce) to avoid per-call closure overhead

Benchmark results

Measured on CPython 3.14.3, Protocol V4, 300 iterations, 100 warmup. All values in nanoseconds per row.

ScenarioMaster (min ns/row)PR (min ns/row)Master (median ns/row)PR (median ns/row)Speedup (min)Speedup (median)
5 int cols, 10 rows26771911319225581.40x1.25x
5 int cols, 100 rows21551489287719081.45x1.51x
5 int cols, 1000 rows26751848316522601.45x1.40x
5 mixed cols, 100 rows26252024322522031.30x1.46x
5 mixed cols, 1000 rows29421926388021181.53x1.83x
10 int cols, 100 rows, 50% NULL46663095528433141.51x1.59x
10 int cols, 1000 rows, 50% NULL48122737615631661.76x1.94x
10 int cols, 100 rows, no NULL50823826533942011.33x1.27x
10 int cols, 1000 rows, no NULL51163647618445891.40x1.35x

1.3x–1.8x speedup on the pure Python path. The speedup is higher with NULL-heavy workloads because the inline path short-circuits from_binary() for negative-length (NULL) columns.

Merge conflict note

⚠️ This PR modifies the same recv_results_rows method as PR #630, which also splits the method into CE/non-CE branches. If both PRs are accepted, there will be a merge conflict requiring manual resolution.

Testing

  • All 651 existing unit tests pass (16 pre-existing skips)
  • Added test for decode error wrapping in the inline path (test_protocol.py)

@mykaul
mykaul marked this pull request as draft March 25, 2026 20:33
@mykaul
mykaulforce-pushed the perf/inline-row-decode branch from 3c3fea8 to 020c764CompareApril 7, 2026 10:57
@mykaulmykaul changed the title perf: inline row decoding and eliminate closures in recv_results_rowsperf: inline row decoding and eliminate closures in recv_results_rows (100's to 1000's of ns, x1.3-1.8 speedup)Apr 7, 2026
@mykaulmykaul changed the title perf: inline row decoding and eliminate closures in recv_results_rows (100's to 1000's of ns, x1.3-1.8 speedup)perf: inline row decoding and eliminate closures in recv_results_rows (100's to 1000's of ns, x1.3-1.8 speedup, Python only)Apr 7, 2026
@mykaul
mykaulforce-pushed the perf/inline-row-decode branch from 020c764 to 30d3a44CompareApril 9, 2026 17:19
Split recv_results_rows into fast path (no column encryption) and slow
path (column encryption enabled):
Fast path (common case):
- Reads raw column bytes and decodes types in a single pass per row
via _decode_row_inline(), eliminating the intermediate list-of-lists
- Skips ColDesc namedtuple creation entirely (only needed for CE)
- No closure allocation per call
- Wraps decode errors with column name/type info for diagnostics
Slow path (column encryption):
- Preserves full CE logic with ColDesc creation
- Moves decode_val/decode_row closures to module-level functions
(_decode_val_ce, _decode_row_ce) to avoid per-call closure overhead
Note: This PR modifies the same method as PR scylladb#630 (which also splits
recv_results_rows into CE/non-CE branches). There will be a merge
conflict that needs manual resolution if both PRs are accepted.
CopilotAI review requested due to automatic review settings July 29, 2026 20:20
@mykaul
mykaulforce-pushed the perf/inline-row-decode branch from 30d3a44 to 8d73498CompareJuly 29, 2026 20:20
@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c697dc74-3211-4d7c-b525-5189e8d082e4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes pure-Python result-row decoding while preserving the column-encryption path.

Changes:

  • Decodes unencrypted rows in one pass without intermediate lists or closures.
  • Moves encrypted-column decoding helpers to module scope.
  • Adds coverage for inline decoding error messages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

FileDescription
cassandra/protocol.pyImplements optimized row decoding paths.
tests/unit/test_protocol.pyTests inline decode error wrapping.

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mykaul