perf: lazy-init _callbacks/_errbacks in ResponseFuture (25ns saving, 2x speedup, 112 bytes saved) - #795
perf: lazy-init _callbacks/_errbacks in ResponseFuture (25ns saving, 2x speedup, 112 bytes saved)#795mykaul wants to merge 1 commit into
Conversation
40c469e to
89dabcf
Compare
Defer list allocation for _callbacks and _errbacks from __init__ to first use in add_callback()/add_errback(). On the synchronous execute path (session.execute()), no callbacks are registered, so both lists are never allocated — saving 112 bytes per request. All access is under _callback_lock; _set_final_result and _set_final_exception use 'or ()' guard to iterate safely when None. Benchmark: 2.2x faster init (0.06 -> 0.03 us), 112 bytes saved/request.
89dabcf to
68ef1ef
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 current Given the recent history of
No unresolved review threads or comments existed on the PR prior to this push. Verified:
Force-pushed the rebased commit (same commit, no content changes needed beyond the rebase) to keep this a single amended commit rather than adding new ones. Still a draft. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR optimizes ResponseFuture by lazily initializing _callbacks/_errbacks to avoid unnecessary list allocations on the synchronous execute path, while keeping callback behavior consistent and thread-safe.
Changes:
- Change
_callbacks/_errbacksinitialization from empty lists toNone, and lazily allocate on firstadd_*use - Guard callback/errback iteration in
_set_final_result/_set_final_exceptionto handleNonesafely - Add unit tests and a micro-benchmark to validate and measure the optimization
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cassandra/cluster.py |
Implements lazy init and safe iteration for callbacks/errbacks in ResponseFuture. |
tests/unit/test_lazy_init_callbacks.py |
Adds focused unit tests validating lazy init and callback/errback behavior. |
benchmarks/bench_lazy_init_callbacks.py |
Adds a micro-benchmark to quantify time and allocation savings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import unittest | ||
| from unittest.mock import Mock, patch, PropertyMock | ||
| from threading import Lock, Event | ||
|
|
||
| from cassandra.cluster import ResponseFuture, _NOT_SET |
Summary
_callbacksand_errbacksfromResponseFuture.__init__()to first use inadd_callback()/add_errback()session.execute()), no callbacks are registered, so both lists are never allocated — saving 112 bytes per request_callback_lock;_set_final_resultand_set_final_exceptionuseor ()guard to iterate safely whenNoneBenchmark
[]x2Nonex2Memory saved per request (no callbacks): 112 bytes (2 x 56-byte empty list)
Tests