(improvement) (Cython only) row_parser: cache ParseDesc for prepared statements (us improvements - 20-100x speedups!)

[mykaul]

Cache the ParseDesc object constructed in recv_results_rows() so that repeated executions of the same prepared statement skip the list comprehensions, ColDesc construction, and make_deserializers() call.

The cache is keyed by id(column_metadata). For prepared statements the result_metadata list is stored on PreparedStatement and reused, so id() is stable. On cache hit we verify object identity (cached_ref is column_metadata) and that session-level settings (column_encryption_policy, protocol_version) still match.

A clear_parse_desc_cache() function is exposed for testing.

Benchmark results (median, pytest-benchmark)

ParseDesc construction only

Columns	Before (original)	After (with cache)	Speedup
5 cols	3,966 ns	191 ns	21x
10 cols	5,730 ns	175 ns	33x
20 cols	9,266 ns	166 ns	56x
50 cols	19,388 ns	193 ns	100x

Full pipeline (ParseDesc + row parsing)

Scenario	Before (original)	After (with cache)	Speedup
1 row × 10 col	6,674 ns	1,418 ns	4.7x
100 rows × 5 col	48,814 ns	43,825 ns	1.1x
1000 rows × 5 col	449,386 ns	430,058 ns	1.04x

For small result sets (single-row lookups common with prepared statements), ParseDesc construction is a large fraction of the total response-path cost. Caching eliminates it entirely after the first execution.

All 116 unit tests pass (1 skipped — pre-existing test_datetype issue).

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce.
• I have adjusted the documentation in ./docs/source/.
• I added appropriate Fixes: annotations to PR description.

[Copilot]

Pull request overview

This PR optimizes the Cython row parsing fast-path for prepared statements by caching the ParseDesc built in recv_results_rows(), reducing repeated per-response construction overhead.

Changes:

• Add a module-level cache in cassandra/row_parser.pyx keyed by id(column_metadata) to reuse ParseDesc, column_names, and column_types for prepared statement executions.
• Expose clear_parse_desc_cache() and call it from Cluster.shutdown().
• Add a benchmark/correctness benchmark module to measure and validate the caching behavior.

Reviewed changes

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

File	Description
cassandra/row_parser.pyx	Introduces ParseDesc caching and adds a public cache-clear helper.
cassandra/cluster.py	Clears the module-level ParseDesc cache during Cluster.shutdown().
benchmarks/test_parse_desc_cache_benchmark.py	Adds benchmarks and cache-behavior checks for the new caching approach.

---

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

[Copilot]

Pull request overview

This PR improves the Cython result-row parsing hot path by caching the ParseDesc constructed in recv_results_rows() for repeated executions of the same prepared statement (stable result_metadata object), reducing repeated metadata processing and deserializer construction.

Changes:

• Add a bounded module-level cache in cassandra/row_parser.pyx keyed by id(column_metadata) with identity + session-settings validation.
• Add cache management helpers (clear_parse_desc_cache(), get_parse_desc_cache_size()) intended for test/benchmark visibility.
• Add a new benchmark module to measure ParseDesc construction and end-to-end parsing impact.

Reviewed changes

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

File	Description
cassandra/row_parser.pyx	Introduces ParseDesc caching for prepared-statement row decoding and adds cache clear/size helpers.
benchmarks/test_parse_desc_cache_benchmark.py	Adds benchmarks (and some correctness assertions) for cached vs uncached ParseDesc construction/parsing.

---

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

[coderabbitai]

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: 6d10e53b-f9af-43af-bb46-450b2e139edd

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.}

[mykaul]

Rebased onto current master (was based on 3.29.9; master has since gained the SCYLLA_USE_METADATA_ID extension / atomic result_metadata_and_id pair work from DRIVER-153). Rebase applied cleanly with no conflicts — this PR only touches cassandra/row_parser.pyx plus benchmark/test files, and row_parser.pyx itself had no upstream changes in that range.

Cache-invalidation correctness on schema change / re-preparation (the main thing I scrutinized): verified safe. Summary:

• The cache key is id(column_metadata), where column_metadata is the exact result_metadata list object snapshotted from PreparedStatement.result_metadata_and_id at bind time.
• PreparedStatement.update_result_metadata() — called both on a METADATA_CHANGED response and after an UNPREPARED re-prepare — always replaces result_metadata with a brand-new list built fresh from the server's response; it never mutates the existing list in place. A schema change therefore always produces a new id(), so the cache misses and rebuilds a ParseDesc with the new column types instead of returning stale ones.
• The cache entry stores a strong reference to the old column_metadata list, so its id() can never be reused by an unrelated object while the entry is still cached — this rules out the classic "id() as a stale memory address" aliasing hazard. The extra cached[0] is column_metadata identity check (plus column_encryption_policy identity and protocol_version equality) is a second line of defense on top of that.
• When a response itself carries fresh inline metadata (self.column_metadata non-None, e.g. exactly the METADATA_CHANGED case), recv_results_rows takes the uncached path (_build_parse_desc) and never touches the cache for that decode — so the response that just informed us of a schema change is always decoded with its own accompanying metadata.
• This matches the existing unit coverage in tests/unit/test_response_future.py / tests/unit/test_query.py (PreparedStatementMetadataPairTest, test_execute_after_prepare_updates_result_metadata_id, etc.) for the atomic-pair mechanism itself, and the integration coverage in tests/integration/standard/test_scylla_metadata_id.py (test_metadata_changed_recovers_after_schema_change, test_conditional_statement_metadata_is_stable_across_outcomes) that exercises an actual ALTER TABLE + re-decode end to end.

I added one small doc-only change (amended into the first commit, no new commit): a comment in row_parser.pyx spelling out that this cache's safety depends on result_metadata only ever being replaced, never mutated in place, so a future refactor of query.py doesn't silently break that invariant.

CI / review status: all 20 checks were green on the pre-rebase commit; no unresolved review threads (Copilot's comments — shutdown-hook global side effect, memory retention, missing unit tests, copyright header — were all addressed in the second commit or by removing the Cluster.shutdown() hook entirely).

Testing performed locally (Cython extensions rebuilt in-place):

• tests/unit/cython/test_parse_desc_cache.py: 8 passed
• tests/unit/test_query.py: 16 passed
• tests/unit/test_response_future.py, test_protocol.py, test_parameter_binding.py: 114 passed
• benchmarks/test_parse_desc_cache_benchmark.py (correctness assertions, --benchmark-disable): 37 passed
• Full tests/unit/: 777 passed, 38 skipped (all pre-existing/environmental — asyncore deprecation, gevent/eventlet not installed, etc.), 0 failed

Force-pushed the rebased branch (2 commits, same as before, amended — no new commits added).

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (11)

tests/unit/cython/test_parse_desc_cache.py:117

• If the Cython cassandra.row_parser import fails (_HAS_ROW_PARSER == False), these tests can still be collected and then fail with NameError / TypeError when calling get_parse_desc_cache_size() or _recv_results_rows. Consider skipping the entire test class when _HAS_ROW_PARSER is false (e.g., setUp() calls self.skipTest(...) when unavailable, or a @unittest.skipUnless(_HAS_ROW_PARSER, ...) decorator).

class ParseDescCacheTest(unittest.TestCase):
    """Tests for the Cython ParseDesc cache in row_parser.pyx."""

    def setUp(self):
        if _HAS_ROW_PARSER:
            clear_parse_desc_cache()

    def tearDown(self):
        if _HAS_ROW_PARSER:
            clear_parse_desc_cache()

    @cythontest
    def test_cache_hit_returns_same_objects(self):

cassandra/row_parser.pyx:63

• The ParseDesc construction logic is duplicated between _get_or_build_parse_desc() (miss path) and _build_parse_desc(). To avoid drift/bugs when this construction changes in the future, consider factoring the shared build logic into a single internal helper (e.g., _construct_parse_desc(...) returning (desc, names, types)), and have both functions call it.

cdef inline tuple _get_or_build_parse_desc(object column_metadata, object column_encryption_policy, int protocol_version):

cassandra/row_parser.pyx:86

• The ParseDesc construction logic is duplicated between _get_or_build_parse_desc() (miss path) and _build_parse_desc(). To avoid drift/bugs when this construction changes in the future, consider factoring the shared build logic into a single internal helper (e.g., _construct_parse_desc(...) returning (desc, names, types)), and have both functions call it.

    # Cache miss -- build everything
    cdef list column_names = [md[2] for md in column_metadata]
    cdef list column_types = [md[3] for md in column_metadata]
    cdef object desc = ParseDesc(
        column_names, column_types, column_encryption_policy,
        [ColDesc(md[0], md[1], md[2]) for md in column_metadata],
        make_deserializers(column_types), protocol_version)

cassandra/row_parser.pyx:100

• The ParseDesc construction logic is duplicated between _get_or_build_parse_desc() (miss path) and _build_parse_desc(). To avoid drift/bugs when this construction changes in the future, consider factoring the shared build logic into a single internal helper (e.g., _construct_parse_desc(...) returning (desc, names, types)), and have both functions call it.

cdef inline tuple _build_parse_desc(object column_metadata, object column_encryption_policy, int protocol_version):

cassandra/row_parser.pyx:110

• The ParseDesc construction logic is duplicated between _get_or_build_parse_desc() (miss path) and _build_parse_desc(). To avoid drift/bugs when this construction changes in the future, consider factoring the shared build logic into a single internal helper (e.g., _construct_parse_desc(...) returning (desc, names, types)), and have both functions call it.

    cdef list column_names = [md[2] for md in column_metadata]
    cdef list column_types = [md[3] for md in column_metadata]
    cdef object desc = ParseDesc(
        column_names, column_types, column_encryption_policy,
        [ColDesc(md[0], md[1], md[2]) for md in column_metadata],
        make_deserializers(column_types), protocol_version)

cassandra/row_parser.pyx:88

• Clearing the entire cache at capacity can cause sudden churn if an application legitimately uses slightly more than 256 prepared statements (repeated full rebuilds after each threshold crossing). A low-overhead alternative is evicting a single entry (e.g., oldest insertion-order key) on overflow, which preserves most cached entries while still avoiding complex LRU bookkeeping on the hot path (eviction only happens on misses).

    # Simple bounded eviction: if the cache is too large, clear it entirely.

cassandra/row_parser.pyx:93

• Clearing the entire cache at capacity can cause sudden churn if an application legitimately uses slightly more than 256 prepared statements (repeated full rebuilds after each threshold crossing). A low-overhead alternative is evicting a single entry (e.g., oldest insertion-order key) on overflow, which preserves most cached entries while still avoiding complex LRU bookkeeping on the hot path (eviction only happens on misses).

    if len(_parse_desc_cache) >= _PARSE_DESC_CACHE_MAX_SIZE:
        _parse_desc_cache.clear()

cassandra/row_parser.pyx:115

• These def functions become part of the public cassandra.row_parser import surface. If they are intended strictly for tests/debugging, consider marking them as internal (e.g., prefix with _) and/or adding explicit documentation that they are not a stable public API. This avoids committing the project to long-term support of testing hooks as user-facing API.

def clear_parse_desc_cache():
    """Clear the ParseDesc cache.  Exposed for testing."""

cassandra/row_parser.pyx:120

• These def functions become part of the public cassandra.row_parser import surface. If they are intended strictly for tests/debugging, consider marking them as internal (e.g., prefix with _) and/or adding explicit documentation that they are not a stable public API. This avoids committing the project to long-term support of testing hooks as user-facing API.

def get_parse_desc_cache_size():
    """Return the current number of entries in the ParseDesc cache.  Exposed for testing."""

benchmarks/test_parse_desc_cache_benchmark.py:43

• Module-level pytest.importorskip('pytest_benchmark') skips all tests in this file (including the non-benchmark correctness tests), which can unintentionally reduce coverage in environments that run pytest without the plugin. Consider limiting the skip to only the benchmark-dependent tests (or splitting correctness checks into tests/ and leaving only benchmarks under benchmarks/).

# Skip the entire module when pytest-benchmark is not installed.

benchmarks/test_parse_desc_cache_benchmark.py:48

• Module-level pytest.importorskip('pytest_benchmark') skips all tests in this file (including the non-benchmark correctness tests), which can unintentionally reduce coverage in environments that run pytest without the plugin. Consider limiting the skip to only the benchmark-dependent tests (or splitting correctness checks into tests/ and leaving only benchmarks under benchmarks/).

pytest.importorskip("pytest_benchmark")