(improvement) cache deserializer instances in find_deserializer and make_deserializers - #6
Open
mykaul wants to merge 119 commits into
Open
(improvement) cache deserializer instances in find_deserializer and make_deserializers#6mykaul wants to merge 119 commits into
mykaul wants to merge 119 commits into
Conversation
mykaulforce-pushed
the
perf/cache-deserializer-lookup
branch
from
March 13, 2026 10:03
c9eb0fe to
5894e22CompareIntroduce the data layer for Private Link client routes support: - ClientRoutesChangeType enum for CLIENT_ROUTES_CHANGE event types - ClientRouteProxy dataclass and ClientRoutesConfig for user-facing configuration - _Route frozen dataclass for immutable route records - _RouteStore for thread-safe route storage with atomic update/merge and preferred route selection that avoids unnecessary connection_id migration when multiple routes exist for the same host
Add _ClientRoutesHandler which manages the full lifecycle of dynamic address translation via system.client_routes: - initialize(): loads all routes at startup and on control connection reconnect - handle_client_routes_change(): processes CLIENT_ROUTES_CHANGE events with targeted merge or full refresh depending on event data - _query_all_routes_for_connections(): complete refresh query using connection_id IN (...) - _query_routes_for_change_event(): targeted query grouping by connection_id with host_id IN (...) per group - _execute_routes_query(): common query execution and result parsing with proxy address override support - resolve_host(): host_id to (address, port) resolution with DNS lookup
- ClientRoutesEndPointFactory: creates endpoints from system.peers rows by extracting host_id, deferring address translation and DNS resolution until connection time - ClientRoutesEndPoint: endpoint that resolves via _ClientRoutesHandler on each connection attempt, ensuring immediate reaction to route changes and CLIENT_ROUTES_CHANGE events
Cluster: - Add client_routes_config parameter with mutual exclusivity check against endpoint_factory - Create _ClientRoutesHandler and ClientRoutesEndPointFactory when client_routes_config is provided ControlConnection: - Register CLIENT_ROUTES_CHANGE event watcher when handler is present - Forward events to handler via _handle_client_routes_change - Trigger full route re-read on control connection reconnection
Cover ClientRouteEntry/ClientRoutesConfig validation, _RouteStore get/merge operations, _ClientRoutesHandler initialization, ClientRoutesEndPoint resolution with and without route mappings, and SSL check_hostname rejection with client_routes_config.
Add comprehensive integration tests covering: - TCP proxy and NLB emulator infrastructure for simulating private link connectivity - query_routes filtering with different connection/host ID combinations - Full private-link connectivity verifying all driver connections go exclusively through the NLB proxy - Dynamic route updates via REST API with driver reconnection through new proxy ports
mykaulforce-pushed
the
perf/cache-deserializer-lookup
branch
from
March 20, 2026 17:31
5894e22 to
44cdd0dCompareRecently scylladb started to rely on the options "--auth-superuser-name" and "--auth-superuser-salted-password" to ensure that a cassandra/cassandra user exists for tests - without those options a default superuser no longer exists.
…ames Skip the regex scanner and stack-based parser in parse_casstype_args() when the type string has no parentheses. For simple types like 'AsciiType' or 'org.apache.cassandra.db.marshal.FloatType', go directly to lookup_casstype_simple() which is just a prefix strip + dict lookup. This avoids re.Scanner, re.split on ':' / '=>', int() try/except, and list-of-lists stack manipulation for the common case of non-parameterized types. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
The time.sleep(10) in setup_keyspace() is redundant because callers already ensure the cluster is fully ready before calling it: - use_cluster() calls start_cluster_wait_for_up() which uses wait_for_binary_proto=True + wait_other_notice=True, then wait_for_node_socket() per node - External cluster path (wait=False) had no sleep anyway Remove the wait parameter entirely and its associated sleep, saving 10s per cluster startup.
Replace fixed sleeps with condition-based polling to speed up tests: - simulacron/utils.py: replace 5s sleep with HTTP endpoint polling (max 15s timeout, typically <1s) - test_authentication.py: replace 10s sleep with auth readiness poll that tries connecting with default credentials - upgrade/__init__.py: replace 10s auth sleep with same polling pattern - upgrade/test_upgrade.py: replace 3x 20s sleeps (60s total) with control connection readiness polling Total potential saving: ~95s of unconditional waiting per test run.
Replace fixed sleeps with condition-based polling in four test files: - test_shard_aware.py: replace 25s of sleeps (5+10+5+5) with wait_until_not_raised polling for reconnection after shard connection close and iptables blocking - test_metrics.py: replace 15s of sleeps (5+5+5) with polling for cluster recovery and node-down detection - test_tablets.py: replace 13s of sleeps (3+10) with polling for metadata refresh and decommission completion - simulacron/test_connection.py: replace 20s of sleeps (10+10) with polling for quiescent pool state Total potential saving: ~73s of unconditional waiting.
… for invalidation The tablet tests were intermittently failing because: 1. get_query_trace() used the default 2s max_wait, which is too short under resource pressure (--smp 2). Increased to 10s. 2. test_tablets_invalidation_decommission_non_cc_node used a fixed time.sleep(2) hoping tablet metadata invalidation would complete. Replaced with wait_until polling for the tablet record to be purged (0.5s delay, 20 attempts = 10s budget).
- test_cluster.py: replace sleep(1) x10 iterations with connect(wait_for_all_pools=True) for deterministic pool readiness - test_query.py: replace sleep(5) with wait_until polling for 'Preparing all known prepared statements' log message - test_connection.py: replace sleep(2) with wait_until polling for host_down listener notification
…superuser config Use set_configuration_options() (the Python API behind `ccm updateconf`) to set auth_superuser_name and auth_superuser_salted_password directly in the YAML config instead of passing them via the SCYLLA_EXT_OPTS environment variable.
…ema_agreement min(self._timeout, total_timeout - elapsed) raises TypeError when control_connection_timeout is set to None, which is explicitly documented as a supported value (meaning no timeout). Guard the min() call so that when self._timeout is None, we use only the remaining schema agreement wait time.
Patch reactor.running to False in setUp() so that maybe_start() always enters the branch that spawns the reactor thread. Without this, leaked global reactor state from prior tests can leave reactor.running as True, causing maybe_start() to skip thread creation and the reactor.run mock to never be called — making the assertion in test_connection_initialization fail intermittently. Observed in CI on PyPy 3.11 + macOS x86 (Rosetta 2), where timing differences make the reactor state leak more likely.
The column kind filter at line 2744 used 'clustering_key' but system_schema.columns uses 'clustering' as the kind value. This caused clustering columns to not be excluded from the 'other columns' loop, resulting in them being processed twice (once as clustering key, once as regular column). The correct value 'clustering' was already used 6 lines above in the clustering key extraction loop.
ScyllaDB doesn't support triggers, so skip the triggers query when connected to ScyllaDB. This is detected by checking if the connection has shard awareness (using the existing _is_not_scylla() method). Changes to both SchemaParserV3 and SchemaParserV4: - Modified _query_all() to conditionally append triggers query only for non-ScyllaDB - Modified _query_all() response unpacking to use array slicing for cleaner code - Modified get_table() in V3 to conditionally query triggers This eliminates unnecessary failed queries to system_schema.triggers on ScyllaDB. Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
- Fix spelling: 'tring' → 'string' in docstring - Remove extra 't' at end of comment - Refactor complex list comprehension for clarity - Use 'is None' instead of '== None' for None comparison Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
Co-authored-by: mykaul <4655593+mykaul@users.noreply.github.com>
mykaulforce-pushed
the
perf/cache-deserializer-lookup
branch
2 times, most recently
from
April 3, 2026 18:53
9dc1ddf to
4a7b199CompareThe pool module emits a DEBUG log message when selecting a connection for a query. Emitting a log message for every query is too noisy. Since Python logging lacks a TRACE level, just remove the log.
The docstring and test promised ValueError for invalid scope values (e.g. 'planet'), but the validation was never implemented in the method body. Add an explicit check against the three SchemaAgreementScope members. Introduced in commit 0d215f4 (cluster: add Session.wait_for_schema_agreement).
Resolves Dependabot alert scylladb#67 (GHSA-w2fm-2cpv-w7v5 / CVE-2026-22815): aiohttp <= 3.13.3 allows unlimited trailer headers, leading to possible uncapped memory usage (CWE-400/CWE-770). Fixed in aiohttp 3.13.4. aiohttp is a transitive runtime dependency pulled in only by the docs toolchain via gremlinpython==3.7.4. Bumped to 3.14.1 (>= 3.13.4 patched).
Statements with SERIAL or LOCAL_SERIAL consistency level are serialized through the Paxos path on the server, but TokenAwarePolicy only checked is_lwt() (from server prepare metadata) when deciding whether to skip replica shuffling. This meant serial-consistency reads could be routed with shuffled replicas instead of the deterministic order needed for optimal Paxos coordination. Now TokenAwarePolicy also checks the statement's consistency level and skips shuffling for SERIAL/LOCAL_SERIAL, matching LWT routing behavior. Fixes: scylladb#886
Add a guard in the retry execution path that prevents any retry policy from downgrading SERIAL/LOCAL_SERIAL to a non-serial consistency level, which would break serial read (Paxos) guarantees. Also add a unit test verifying DowngradingConsistencyRetryPolicy does not downgrade serial consistency on read timeout or unavailable. Fixes: https://scylladb.atlassian.net/browse/DRIVER-613
test_watchers_are_finished calls libev__cleanup(), which stops the shared libev loop preparer. If a timer test runs later, the stopped preparer means timers are never scheduled and the test can hang. Restore _global_loop._shutdown and restart _global_loop._preparer after the cleanup path runs. Put the restoration in a finally block so the shared loop is left usable even if the post-cleanup watcher assertions fail.
Python 3.12 removed distutils from the standard library. Some Cython test helpers import pyximport, which still imports distutils through setuptools' compatibility shim during collection. Add setuptools to the dev test dependencies so those tests can collect in cibuildwheel's test environment.
Tests skip themselves when their requirements are missing (a library is absent, the wrong event loop is selected, the C extensions aren't built). That is convenient locally but a footgun in CI, where a test may be silently skipped because a dependency was not installed. The libev unit tests were effectively not running in any CI configuration. Add a CASS_DRIVER_NO_SKIP-gated pytest hook in tests/conftest.py that turns skips into failures (xfail untouched), and enable it in the cibuildwheel test-commands where the C extensions are mandatory (Linux/macOS). Tests that genuinely cannot run in the default configuration are listed explicitly via -k/--ignore (reactor tests run separately per EVENT_LOOP_MANAGER; asyncore, column_encryption and a few upstream-disabled/flaky tests excluded). Add -v to every pytest invocation and install the compress-lz4 extra so the lz4 tests actually run. Pass --import-mode=append in the cibuildwheel pytest commands so the installed compiled wheel takes precedence on sys.path over the in-tree pure-Python cassandra source during wheel tests. Keep the global pytest addopts unchanged so local pytest runs keep their normal import behavior. Windows and PyPy keep no-skip off: the extensions are optional on Windows and are never built on PyPy (setup.py forces is_pypy to skip libev/cmurmur3/Cython), so their extension-dependent skips are legitimate. The PyPy override also drops the compress-lz4 extra (no prebuilt PyPy lz4 wheel), uses cross-shell quoting, and deselects the known PyPy/Windows/macOS-incompatible tests.
The final callback was invoked with host_errors (the errors from only the last pool to finish) instead of the accumulated errors dict. If the last pool succeeded, failures from other pools were silently lost. Pass the aggregated errors dict, matching the method's docstring.
The atexit subprocess test inserted the project root at sys.path[0], which shadows the installed compiled wheel with the in-tree pure-Python source. Under cibuildwheel that source lacks the libev C extension, so the import failed and the subprocess produced no output. Append the project path instead so the installed wheel takes precedence, falling back to the source tree only when the driver is not installed. Also assert the subprocess return code and include stdout/stderr in the failure message so future subprocess import/runtime failures are easier to diagnose.
Bumps [soupsieve](https://github.com/facelessuser/soupsieve) from 2.8.3 to 2.8.4. - [Release notes](https://github.com/facelessuser/soupsieve/releases) - [Commits](facelessuser/soupsieve@2.8.3...2.8.4) --- updated-dependencies: - dependency-name: soupsieve dependency-version: 2.8.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Make ProtocolFeatures.__init__ keyword-only and build it by keyword in parse_from_supported. Independently developed protocol extensions (SCYLLA_USE_METADATA_ID, TABLETS_ROUTING_V2) each add fields to this class; keyword construction lets them do so without conflicting over positional-argument order. All existing callers already used keywords.
Connection.send_msg now passes the connection's negotiated ProtocolFeatures to the encoder; _ProtocolHandler.encode_message accepts it as a new required protocol_features argument (passed by keyword from send_msg) and forwards it to every message's send_body, which gains the same parameter. Messages carry connection-independent request data; send_body decides the wire format from (protocol_version, protocol_features), so fields belonging to a negotiated protocol extension are emitted exactly on the connections that negotiated it — on every send path, including the control-connection fallback, and without mutating shared message objects per attempt. This is pure plumbing: no message consumes the parameter yet, so no bytes on the wire change. It is groundwork for the SCYLLA_USE_METADATA_ID (scylladb#770) and TABLETS_ROUTING_V2 (scylladb#913) extensions, which must serialize extension fields based on what the serving connection negotiated. The encode side becomes symmetric with decode_message, which already receives protocol_features. This changes the contracted signature of encode_message: custom protocol handlers overriding it must accept the protocol_features keyword argument. The argument is deliberately required, with no default and no fallback for old-style encoders: extensions are negotiated per connection at STARTUP before the per-request handler is known, so an encoder unaware of protocol_features could silently omit fields a negotiated extension requires; omitting it fails fast with TypeError instead. Tests: send_msg hands the connection's features to the encoder; encode_message forwards them into send_body (plain and compressed paths) and raises TypeError when the argument is omitted; a byte-identity suite pins frames for representative messages (v3/v4/v5) to the exact bytes produced before this change, both without features and with all-default features. Co-authored-by: Dawid Mędrek <dawid.medrek@scylladb.com>
Note in the protocol API docs that both contracted _ProtocolHandler methods receive the connection's negotiated ProtocolFeatures, spelling out the calling conventions: decode_message receives it positionally, encode_message as the required protocol_features keyword argument, so overrides must keep that parameter name. Add a CHANGELOG entry with upgrade guidance for custom protocol handlers (accept protocol_features, prefer **kwargs for future-proofing, forward it when delegating to send_body).
Problem: shard-aware endpoint selection treated legacy ssl_options as SSL-enabled only when the dict was truthy. An explicit empty ssl_options={} was therefore handled like plaintext and could select the non-SSL shard-aware port, diverging from the cluster-level SSL-enabled check.
Fix: treat SSL as enabled when ssl_context is set or ssl_options is not None. SSL-enabled configurations now use the SSL shard-aware port when advertised and otherwise fall back to regular non-shard-aware connections instead of the plaintext shard-aware port. Unit coverage now includes ssl_context, non-empty ssl_options, and empty ssl_options.Implement the SCYLLA_USE_METADATA_ID protocol extension, which backports the CQL v5 prepared-statement metadata-id mechanism to earlier protocol versions. When negotiated, the server includes a hash of the result metadata in the PREPARE response; the driver sends it back with every EXECUTE, allowing the server to omit result metadata from responses (skip_meta) and to report schema changes with METADATA_CHANGED plus fresh metadata, which the driver adopts automatically. protocol_features.py: parse the extension from SUPPORTED, echo it in STARTUP, expose it as ProtocolFeatures.use_metadata_id. protocol.py: ExecuteMessage carries connection-independent request data (skip_meta, result_metadata_id) fixed at construction; serialization decides the wire format from the (protocol_version, protocol_features) that Connection.send_msg supplies for the serving connection: - The metadata-id field is written iff the connection speaks CQL v5+ or negotiated the extension - always, on such connections. An empty sentinel (b'') is written when the statement has no id (prepared before the extension was active, e.g. during a rolling upgrade, or an LWT statement): the sentinel mismatch makes the server respond with METADATA_CHANGED plus the current id and metadata, so such statements acquire an id on their first execution. This also fixes a TypeError on v5 when result_metadata_id was None. - _SKIP_METADATA_FLAG is written only when the SCYLLA_USE_METADATA_ID extension is negotiated on the connection; without the metadata-id mechanism a schema change after PREPARE would leave the driver decoding rows with stale cached metadata. This is deliberately narrower than the metadata-id field above: on native CQL v5 the field is part of the frame layout, but the driver does not request skip there. Upstream never emitted _SKIP_METADATA_FLAG on any version (_write_query_params never wrote it), and enabling the skip optimization for native v5 is a separate change kept out of scope for this Scylla extension. Because messages are immutable after construction, every send path is correct without per-path setup - including the control-connection fallback - and concurrent sends of the same message (speculative executions) cannot race on per-connection state. query.py: PreparedStatement stores (result_metadata, result_metadata_id) as one tuple replaced in a single attribute assignment, read through compatibility properties and updated via update_result_metadata(). Response callbacks update statements while request threads read them; a torn pair (fresh id + stale metadata) would make the server skip sending metadata while rows are decoded against the wrong columns, with no recovery. The compatibility setters are documented as non-atomic relative to each other - update_result_metadata() is the atomic path; the setters exist only for callers assigning the old individual attributes. cluster.py: _create_response_future snapshots the pair once and requests skip_meta only when the statement has both an id and usable cached metadata (result_metadata is None for NO_METADATA/LWT statements and [] for zero-column statements; neither can nor needs to skip metadata). The same snapshot is handed to the ResponseFuture, so a skip_meta response is decoded against the metadata that pairs with the id the message sent - not a later re-read of the statement cache, which a concurrent METADATA_CHANGED could have replaced between construction and send (and which also keeps speculative sends of one message internally consistent). _set_result adopts a METADATA_CHANGED response by replacing the pair atomically; a response carrying a new id without column metadata is ignored with a warning, since adopting the id alone would create the unrecoverable stale-decode state. skip_meta additionally stays off for continuous paging (@dkropachev): Connection.process_msg hardcodes result_metadata=None for every page after the first, since it isn't threaded through the paging session - a skip_meta response has nothing to decode page 2+ against, and would crash on it. _execute_after_prepare refreshes the pair from exactly what the reprepare response carries, including the id (@dkropachev): falling back to the previously cached id when the response has none risks pairing it with metadata from a different schema version than the one that id was computed for - e.g. if the schema changed and then reverted between the two PREPAREs, the old id can become valid again for the current schema while paired locally with an intermediate version's metadata, with no server-side mismatch to catch it. Dropping it instead lets the next id-aware execute re-acquire a correctly paired id through the same b'' sentinel self-healing path a never-prepared statement uses. docs/scylla-specific.rst: documents the extension and its behaviour, worded so the skip_meta optimization reads as conditional on the extension being negotiated rather than pre-existing default behaviour. CHANGELOG.rst: add a Features entry for the extension.
Unit tests for the extension across its layers: test_protocol_features.py: SCYLLA_USE_METADATA_ID parsed from SUPPORTED and echoed in STARTUP options; absent by default. test_protocol.py (wire format): - metadata-id field written on v4 iff the connection negotiated the extension, with the exact bytes asserted; empty sentinel (b'') when the statement has no id, on both the extension path (v4) and the v5 native path (previously a TypeError); - _SKIP_METADATA_FLAG written when skip_meta is requested and the SCYLLA_USE_METADATA_ID extension is negotiated (v4 or v5), and NOT set on a native v5 connection without the extension (the id field is still written there, but the driver does not request skip); also suppressed - together with the id field - on a v4 connection without the extension, even when the statement carries an id; - PREPARED response decoding reads result_metadata_id iff the extension was negotiated (or v5); METADATA_CHANGED/NO_METADATA flag handling. test_query.py: PreparedStatement stores the (result_metadata, result_metadata_id) pair atomically - constructor, update_result_metadata, and the backwards-compatible single-attribute setters all replace the pair as one unit, and previously-taken snapshots stay internally consistent. test_response_future.py: - _create_response_future builds ExecuteMessage from a single pair snapshot: skip_meta only with both an id and usable cached metadata; disabled for id-less statements, NO_METADATA/LWT statements (result_metadata None) and zero-column statements (result_metadata []), while the id still rides on the message; - _query sends the message exactly as constructed (no per-connection mutation - regression test for the speculative-execution race) and decodes a skip_meta response against the metadata snapshotted when the message was built, not a later read of the statement cache (regression for a concurrent METADATA_CHANGED racing the send); - _set_result METADATA_CHANGED path replaces the cached pair atomically; a response with a new id but no column metadata (empty or absent) is ignored with a warning, leaving the cached pair unchanged - adopting the id alone would poison the cache with a stale-metadata/current-id pair the server would never refresh; - _execute_after_prepare refreshes the pair from exactly what the reprepare response carries, including the id, and no longer keeps the previous id when the response has none (@dkropachev: doing so risked pairing a stale id with metadata from a different schema version - test_execute_after_prepare_no_metadata_id_in_response_clears_id); - a statement with valid cached metadata+id must still get skip_meta=False when continuous_paging_options is set (@dkropachev: Connection.process_msg hardcodes result_metadata=None for paging-session pages after the first, so a skip_meta response would crash decoding them - test_create_execute_message_continuous_paging_disables_skip_meta). tests/integration/standard/test_scylla_metadata_id.py: live-server coverage against a real Scylla node via CCM, closing the one gap unit tests can't - whether Scylla actually treats the empty result_metadata_id sentinel as a mismatch rather than a protocol error. Confirms extension negotiation, the normal METADATA_CHANGED-after-ALTER-TABLE path, and the sentinel round trip: a statement forced back to result_metadata_id=None (simulating one prepared before the extension was known, e.g. mid rolling-upgrade) executes without error and comes back with a fresh id. Mirrors the equivalent live test already merged in the Java driver (scylladb/java-driver#758, should_handle_empty_metadata_id_when_executing_statement_when_supported). Run locally against Scylla 2026.1.9 via CCM; see PR description for setup and log excerpt.
mykaulforce-pushed
the
perf/cache-deserializer-lookup
branch
from
July 29, 2026 20:25
4a7b199 to
e5fe679CompareUpdates docs theme to 1.9.3.
This was kept this way to preserve legacy behavior, but I think changing the behavior will be less of a problem than what the current behavior causes. The policy is used for reconnections (for example, reconnecting control connection). If reconnect policy finishes generation (it will do so after 64 attempts before my change), then the reconnector finish and the driver won't attempt reconnection anymore. This would be a terrible situation.
This better conveys what this is: not a timeut duration from config, but how much of this timeout is left right now.
The timeout argument in `wait` tells how much we need to wait taking into consideration that we already waited for some other futures. The total wait time that this future had available to complete is different: it includes time we spent waiting for other futures. This created confusing hearbeat messages, that could even show negative wait times. I fixed it by putting both timeouts in the error message. The `timeout` parameter of `OperationTimedOut` I changed to the original timeout because I think it is more useful and relevant here.
…ake_deserializers Cache find_deserializer() and make_deserializers() results in Cython cdef dict caches keyed on cqltype objects to avoid repeated class lookups and Deserializer object creation on every result set. Using cqltype objects (not id()) as cache keys holds strong references, preventing GC/id-reuse correctness issues with parameterized types.
- Add 256-entry size cap to both _deserializer_cache and _make_deserializers_cache to prevent unbounded growth from non-interned parameterized types in unprepared queries. - Add clear_deserializer_caches() public API so that runtime Des* class overrides (e.g. DesBytesType = DesBytesTypeByteArray for cqlsh) can flush stale cached instances. - Add get_deserializer_cache_sizes() diagnostic helper. - Document override/cache interaction in code comments. - Fix benchmark copyright (DataStax -> ScyllaDB), add pytest.importorskip guards for pytest-benchmark and Cython. - Add 11 unit tests for cache hit/miss, clear, eviction bounds, and size reporting. - Add clear_deserializer_caches() calls to integration test for DesBytesType override. - Add regression tests proving VectorType parameterizations that differ only by subtype (e.g. VectorType<float> vs VectorType<smallint>) get distinct, non-colliding cache entries: apply_parameters() never interns, so each is a fresh class object, and find_deserializer()/ make_deserializers() key on that object's identity rather than its name (VectorType's generated __name__ does not encode the subtype, only vector_size, so a name-based cache key would have been unsafe here).
mykaulforce-pushed
the
perf/cache-deserializer-lookup
branch
from
July 31, 2026 18:32
e5fe679 to
91549a0Compare_deserialize_map's `result` and DesTupleType.deserialize's `values` are assigned but never read; both allocate a Python list per call for nothing. Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
find_deserializer()andmake_deserializers()results in Cythoncdef dictcaches to avoid repeated class lookups and Deserializer object creation on every result setid()) as cache keys — holds strong references preventing GC/id-reuse correctness issues with parameterized typesMotivation
On every result set,
make_deserializers(coltypes)is called fromrow_parser.pyx:37, which in turn callsfind_deserializer()for each column type. These functions perform class name lookups andissubclass()chains, then create fresh Deserializer objects — all redundant work when the same column types appear repeatedly (which is always the case for prepared statements).Benchmark results
Benchmarks compare the original code (Before) against the new cached implementation (After).
find_deserializer(single type lookup):make_deserializers(5 types):make_deserializers(10 types):Design notes
cdef dict(C-level, not accessible from Python) for minimal overheadid(cqltype)— holds strong references preventing GC andid()reuseTests
All existing unit tests pass (108 passed, 1 skipped).