(improvement) (python code path only): cache namedtuple class in named_tuple_factory to avoid … (3.x to 100x improvement when cache is used - us improvements!) - #740
Conversation
mykaul
changed the title
9a76016 to
a662281
Compare
mykaul
changed the title
a662281 to
06f7051
Compare
|
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 latest Cache-key correctness (verified, no bug found): the key is
Added regression tests for the case- and order-sensitivity guarantees. Unbounded growth risk (real, fixed): the original version cached forever with no eviction. For a fixed set of prepared statements this is naturally bounded, but an application executing many distinct ad hoc queries against highly variable/generated schemas could grow the cache without bound. Added a cap ( Testing: full No open review threads to resolve. Still a draft — happy to un-draft on request. |
There was a problem hiding this comment.
Pull request overview
Caches generated Row namedtuple classes to reduce repeated result-processing overhead.
Changes:
- Adds a bounded FIFO namedtuple-class cache.
- Adds correctness and performance benchmarks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
cassandra/query.py |
Implements namedtuple-class caching and eviction. |
benchmarks/test_named_tuple_factory_benchmark.py |
Adds cache correctness checks and benchmarks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…repeated exec() calls Cache the Row namedtuple class keyed on tuple(colnames) so Python's namedtuple() (which internally calls exec()) is only invoked once per unique column schema. For prepared statements the column names never change, eliminating redundant class creation on every result set. Cache is a plain dict keyed on tuple(colnames) (raw column names before cleaning, exact names, exact order). Since cleaning/sanitizing colnames is a pure function of that key, two schemas only ever share a cached class when their column names match exactly -- differing case, order, or contents all produce distinct keys. Error handling paths (SyntaxError, Exception) preserved unchanged. The cache is naturally bounded by the number of distinct queries for typical usage (a fixed set of prepared statements), but applications that build many ad hoc queries against highly variable or generated schemas could otherwise grow it without bound. It is now capped at 10000 entries with oldest-first (FIFO) eviction once full, relying on dict insertion order, to keep worst-case memory bounded. Address review feedback: - Thread safety: the miss/evict/insert sequence on a cache miss was not synchronized, so one thread's next(iter(_named_tuple_cache)) (picking an eviction victim) could race with another thread mutating the same dict, raising `RuntimeError: dictionary changed size during iteration`; two threads observing the cache under its bound before either inserted could also together push it past that bound. This driver advertises free-threaded Python support, so the whole check-evict-insert sequence is now guarded by a lock (_named_tuple_cache_lock), with the cache-HIT path left lock-free. Verified with a real pre-fix/post-fix repro under CPython's free-threaded (3.14t) build: the pre-fix code reliably raised RuntimeError and exceeded its stated bound under concurrent misses, the post-fix code did neither across repeated runs. Added a regression test (tests/unit/test_row_factories.py, TestNamedTupleFactoryCacheThreadSafety) that hammers the cache from many threads with distinct column-name sets to force evictions, asserting no exception is raised and the bound is respected. - Test placement: the cache correctness tests (cache-hit, cache-key, eviction) were previously only under benchmarks/, which is not run by the project's wheel test commands (only tests/unit is). Moved them into tests/unit/test_row_factories.py so regressions in the production cache are actually caught in CI; benchmarks/ now contains only the timing benchmarks. - Dev dependencies: added pytest-benchmark to the `dev` dependency group in pyproject.toml so `pytest benchmarks/` is runnable in the documented development environment.
06f7051 to
ef0bb90
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
cassandra/query.py:190
- This non-reentrant lock remains held while
namedtuplecreation and the fallback error handling run. The fallback callslog.warning(and theSyntaxErrorpath callswarnings.warn), both of which execute user-configurable handlers; if a handler re-enters this row factory, it blocks forever on the same lock, and cross-thread logging can also create a lock-order deadlock. Build the candidate class and emit diagnostics before acquiring this lock, then lock only to re-check the key and perform eviction/insertion; concurrent misses may do duplicate construction, but callbacks no longer run inside the critical section.
with _named_tuple_cache_lock:
try:
Row = _named_tuple_cache[key]
except KeyError:
cassandra/query.py:178
- Materializing
colnamesonly for the cache key consumes one-shot iterables, but the miss path then iterates the original object again. For example,named_tuple_factory(iter(("a", "b")), [(1, 2)])now builds a zero-fieldRowand fails, whereas the previous implementation accepted it. Reuse the materialized tuple for the rest of the function so key construction does not change the input semantics.
key = tuple(colnames)
Cache the Row namedtuple class keyed on tuple(colnames) so Python's namedtuple() (which internally calls exec()) is only invoked once per unique column schema. For prepared statements the column names never change, eliminating redundant class creation on every result set.
Motivation
named_tuple_factory is the default row_factory in the driver. Every call to namedtuple('Row', columns) internally calls exec() to generate a new class -- this is surprisingly expensive. For prepared statements executing the same query repeatedly, the column names never change, yet we pay the namedtuple() + exec() cost on every result set.
Benchmark results
All timings in ns (nanoseconds). 500 iterations, 100 warmup, CPython 3.14.3.
10 columns, 1 row (isolates class creation overhead):
5 columns, 100 rows:
10 columns, 100 rows:
5 columns, 1000 rows:
10 columns, 1000 rows:
20 columns, 100 rows:
The speedup is most dramatic when row count is low (the namedtuple class creation cost dominates). With 1000 rows, the per-row tuple construction dominates and the speedup converges to ~1.2x.
Design notes
Tests
All existing unit tests pass (46 passed).
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.