Skip to content

Commit 014e82e

Browse files
committed
perf: skip lock acquisition when no orphaned requests in process_msg
Check orphaned_request_ids truthiness before acquiring the lock. Since orphaned requests are rare (only on timeouts), the set is almost always empty. Skipping the lock in the common case saves ~57 ns per response. The unlocked truthiness check on a set is thread-safe under the GIL. Worst case (false positive): we enter the lock block and re-check, which is correct. Worst case (false negative): an orphaned response is processed normally — acceptable behavior. Benchmark (2M iters, Python 3.14): Empty set (common): 80.6 -> 23.2 ns (3.47x, -57.4 ns/response) Non-empty set (rare): 79.7 -> 87.8 ns (+8.1 ns overhead)
1 parent 5447fbf commit 014e82e

2 files changed

Lines changed: 116 additions & 5 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Copyright ScyllaDB, Inc.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""
16+
Micro-benchmark: orphaned request lock skip in process_msg.
17+
18+
Measures the cost of always acquiring a lock vs checking the set first.
19+
20+
Run:
21+
python benchmarks/bench_orphan_lock_skip.py
22+
"""
23+
24+
import sys
25+
import timeit
26+
from threading import Lock
27+
28+
29+
def bench():
30+
n = 2_000_000
31+
lock = Lock()
32+
orphaned_set = set() # empty — the common case
33+
stream_id = 42
34+
in_flight = 100
35+
36+
# Old: always acquire lock
37+
def old_check():
38+
nonlocal in_flight
39+
with lock:
40+
if stream_id in orphaned_set:
41+
in_flight -= 1
42+
orphaned_set.remove(stream_id)
43+
44+
# New: check set first, skip lock if empty
45+
def new_check():
46+
nonlocal in_flight
47+
if orphaned_set:
48+
with lock:
49+
if stream_id in orphaned_set:
50+
in_flight -= 1
51+
orphaned_set.remove(stream_id)
52+
53+
print(f"=== orphaned request lock skip ({n:,} iters) ===\n")
54+
55+
# Warmup
56+
for _ in range(10000):
57+
old_check()
58+
new_check()
59+
60+
t_old = timeit.timeit(old_check, number=n)
61+
t_new = timeit.timeit(new_check, number=n)
62+
ns_old = t_old / n * 1e9
63+
ns_new = t_new / n * 1e9
64+
saving = ns_old - ns_new
65+
speedup = ns_old / ns_new if ns_new > 0 else float('inf')
66+
print(f" Empty orphaned set (common case):")
67+
print(f" Old (always lock): {ns_old:.1f} ns")
68+
print(f" New (check first): {ns_new:.1f} ns")
69+
print(f" Saving: {saving:.1f} ns ({speedup:.2f}x)")
70+
71+
# Non-empty set (rare case) — should still work
72+
orphaned_set_full = {99, 100, 101}
73+
def old_check_full():
74+
nonlocal in_flight
75+
with lock:
76+
if stream_id in orphaned_set_full:
77+
in_flight -= 1
78+
orphaned_set_full.remove(stream_id)
79+
80+
def new_check_full():
81+
nonlocal in_flight
82+
if orphaned_set_full:
83+
with lock:
84+
if stream_id in orphaned_set_full:
85+
in_flight -= 1
86+
orphaned_set_full.remove(stream_id)
87+
88+
for _ in range(10000):
89+
old_check_full()
90+
new_check_full()
91+
92+
t_old = timeit.timeit(old_check_full, number=n)
93+
t_new = timeit.timeit(new_check_full, number=n)
94+
ns_old = t_old / n * 1e9
95+
ns_new = t_new / n * 1e9
96+
diff = ns_new - ns_old
97+
print(f"\n Non-empty orphaned set (rare case):")
98+
print(f" Old (always lock): {ns_old:.1f} ns")
99+
print(f" New (check first): {ns_new:.1f} ns")
100+
print(f" Overhead: {diff:.1f} ns (extra truthiness check)")
101+
102+
103+
if __name__ == "__main__":
104+
print(f"Python {sys.version}\n")
105+
bench()

cassandra/connection.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,11 +1395,17 @@ def process_msg(self, header, body):
13951395
result_metadata = None
13961396
else:
13971397
need_notify_of_release = False
1398-
with self.lock:
1399-
if stream_id in self.orphaned_request_ids:
1400-
self.in_flight -= 1
1401-
self.orphaned_request_ids.remove(stream_id)
1402-
need_notify_of_release = True
1398+
# Fast path: skip lock when no orphaned requests (common case).
1399+
# Reading orphaned_request_ids without the lock is safe: it's a
1400+
# set and we only check truthiness. A false negative just means
1401+
# we'll process the orphaned response normally; a false positive
1402+
# (rare) falls through to the locked check which is correct.
1403+
if self.orphaned_request_ids:
1404+
with self.lock:
1405+
if stream_id in self.orphaned_request_ids:
1406+
self.in_flight -= 1
1407+
self.orphaned_request_ids.remove(stream_id)
1408+
need_notify_of_release = True
14031409
if need_notify_of_release and self._on_orphaned_stream_released:
14041410
self._on_orphaned_stream_released()
14051411

0 commit comments

Comments
 (0)