Skip to content

Commit 0a46094

Browse files
mykaulmykaul
committed
perf: check isinstance(ResultMessage) first in _set_result for direct attr access
Move the isinstance(response, ResultMessage) check to the top of _set_result so the hot path (successful query results) uses direct attribute access instead of getattr() with defaults. ResultMessage has trace_id, warnings, and custom_payload in __slots__, always initialised in __init__, so direct access is safe and avoids the overhead of 3 getattr() calls + 1 isinstance() that was previously done unconditionally before the type dispatch. For the cold ErrorMessage path, getattr() is still used because ErrorMessage does not declare trace_id in __slots__ or __init__. ConnectionException and generic Exception branches explicitly clear _warnings/_custom_payload to avoid stale values from prior retries. Benchmark (best-of-7, 500k iterations, Python 3.14, pure-Python): _set_result ROWS hot path (no tablets, no tracing): Before: 326 ns After: 271 ns (-55 ns, 1.20x)
1 parent 2818aef commit 0a46094

2 files changed

Lines changed: 51 additions & 25 deletions

File tree

‎cassandra/cluster.py‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4725,28 +4725,32 @@ def _set_result(self, host, connection, pool, response):
47254725
if pool and not pool.is_shutdown:
47264726
pool.return_connection(connection)
47274727

4728-
trace_id = getattr(response, 'trace_id', None)
4729-
if trace_id:
4730-
if not self._query_traces:
4731-
self._query_traces = []
4732-
self._query_traces.append(QueryTrace(trace_id, self.session))
4733-
4734-
self._warnings = getattr(response, 'warnings', None)
4735-
self._custom_payload = getattr(response, 'custom_payload', None)
4736-
4737-
if self._custom_payload and self.session.cluster.control_connection._tablets_routing_v1:
4738-
info = self._custom_payload.get('tablets-routing-v1')
4739-
if info is not None:
4740-
ctype = ResponseFuture._TABLET_ROUTING_CTYPE
4741-
if ctype is None:
4742-
ctype = types.lookup_casstype('TupleType(LongType, LongType, ListType(TupleType(UUIDType, Int32Type)))')
4743-
ResponseFuture._TABLET_ROUTING_CTYPE = ctype
4744-
first_token, last_token, tablet_replicas = ctype.from_binary(info, self.session.cluster.protocol_version)
4745-
tablet = Tablet.from_row(first_token, last_token, tablet_replicas)
4746-
if tablet is not None:
4747-
self.session.cluster.metadata._tablets.add_tablet(self.query.keyspace, self.query.table, tablet)
4748-
47494728
if isinstance(response, ResultMessage):
4729+
# Hot path: ResultMessage has trace_id, warnings, and
4730+
# custom_payload in __slots__, always initialised in __init__,
4731+
# so direct attribute access is safe and faster than getattr().
4732+
trace_id = response.trace_id
4733+
if trace_id:
4734+
if not self._query_traces:
4735+
self._query_traces = []
4736+
self._query_traces.append(QueryTrace(trace_id, self.session))
4737+
4738+
self._warnings = response.warnings
4739+
custom_payload = response.custom_payload
4740+
self._custom_payload = custom_payload
4741+
4742+
if custom_payload and self.session.cluster.control_connection._tablets_routing_v1:
4743+
info = custom_payload.get('tablets-routing-v1')
4744+
if info is not None:
4745+
ctype = ResponseFuture._TABLET_ROUTING_CTYPE
4746+
if ctype is None:
4747+
ctype = types.lookup_casstype('TupleType(LongType, LongType, ListType(TupleType(UUIDType, Int32Type)))')
4748+
ResponseFuture._TABLET_ROUTING_CTYPE = ctype
4749+
first_token, last_token, tablet_replicas = ctype.from_binary(info, self.session.cluster.protocol_version)
4750+
tablet = Tablet.from_row(first_token, last_token, tablet_replicas)
4751+
if tablet is not None:
4752+
self.session.cluster.metadata._tablets.add_tablet(self.query.keyspace, self.query.table, tablet)
4753+
47504754
if response.kind == RESULT_KIND_SET_KEYSPACE:
47514755
session = getattr(self, 'session', None)
47524756
# since we're running on the event loop thread, we need to
@@ -4780,6 +4784,18 @@ def _set_result(self, host, connection, pool, response):
47804784
else:
47814785
self._set_final_result(response)
47824786
elif isinstance(response, ErrorMessage):
4787+
# Cold path: ErrorMessage inherits from _MessageType which
4788+
# defines warnings/custom_payload as class-level defaults but
4789+
# does NOT have trace_id -- getattr is required here.
4790+
trace_id = getattr(response, 'trace_id', None)
4791+
if trace_id:
4792+
if not self._query_traces:
4793+
self._query_traces = []
4794+
self._query_traces.append(QueryTrace(trace_id, self.session))
4795+
4796+
self._warnings = getattr(response, 'warnings', None)
4797+
self._custom_payload = getattr(response, 'custom_payload', None)
4798+
47834799
retry_policy = self._retry_policy
47844800

47854801
if isinstance(response, ReadTimeoutErrorMessage):
@@ -4858,6 +4874,10 @@ def _set_result(self, host, connection, pool, response):
48584874

48594875
self._handle_retry_decision(retry, response, host)
48604876
elif isinstance(response, ConnectionException):
4877+
# ConnectionException has no trace_id/warnings/custom_payload;
4878+
# clear any stale values from a previous retry attempt.
4879+
self._warnings = None
4880+
self._custom_payload = None
48614881
if self._metrics is not None:
48624882
self._metrics.on_connection_error()
48634883
if not isinstance(response, ConnectionShutdown):
@@ -4867,6 +4887,8 @@ def _set_result(self, host, connection, pool, response):
48674887
self.query, cl, error=response, retry_num=self._query_retries)
48684888
self._handle_retry_decision(retry, response, host)
48694889
elif isinstance(response, Exception):
4890+
self._warnings = None
4891+
self._custom_payload = None
48704892
if hasattr(response, 'to_exception'):
48714893
self._set_final_exception(response.to_exception())
48724894
else:

‎tests/unit/test_response_future.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ def make_response_future(self, session):
6161
return ResponseFuture(session, message, query, 1)
6262

6363
def make_mock_response(self, col_names, rows):
64-
return Mock(spec=ResultMessage, kind=RESULT_KIND_ROWS, column_names=col_names, parsed_rows=rows, paging_state=None, col_types=None)
64+
return Mock(spec=ResultMessage, kind=RESULT_KIND_ROWS, column_names=col_names, parsed_rows=rows,
65+
paging_state=None, col_types=None, trace_id=None, warnings=None, custom_payload=None)
6566

6667
def test_result_message(self):
6768
session = self.make_basic_session()
@@ -104,7 +105,8 @@ def test_set_keyspace_result(self):
104105

105106
result = Mock(spec=ResultMessage,
106107
kind=RESULT_KIND_SET_KEYSPACE,
107-
results="keyspace1")
108+
results="keyspace1",
109+
trace_id=None, warnings=None, custom_payload=None)
108110
rf._set_result(None, None, None, result)
109111
rf._set_keyspace_completed({})
110112
assert not rf.result()
@@ -118,7 +120,8 @@ def test_schema_change_result(self):
118120
'keyspace': "keyspace1", "table": "table1"}
119121
result = Mock(spec=ResultMessage,
120122
kind=RESULT_KIND_SCHEMA_CHANGE,
121-
schema_change_event=event_results)
123+
schema_change_event=event_results,
124+
trace_id=None, warnings=None, custom_payload=None)
122125
connection = Mock()
123126
rf._set_result(None, connection, None, result)
124127
session.submit.assert_called_once_with(ANY, ANY, rf, connection, **event_results)
@@ -127,7 +130,8 @@ def test_other_result_message_kind(self):
127130
session = self.make_session()
128131
rf = self.make_response_future(session)
129132
rf.send_request()
130-
result = Mock(spec=ResultMessage, kind=999, results=[1, 2, 3])
133+
result = Mock(spec=ResultMessage, kind=999, results=[1, 2, 3],
134+
trace_id=None, warnings=None, custom_payload=None)
131135
rf._set_result(None, None, None, result)
132136
assert rf.result()[0] == result
133137

0 commit comments

Comments
 (0)