perf: Skip redundant __init__ assignments and remove dead attributes in ResponseFuture (-32ns/call - 4.7% improvement for the common case, +43ns (+4.5%) for the non-common case, - code cleanup also)#806
Draft
mykaul wants to merge 2 commits intoscylladb:masterfrom
Draft
Conversation
…in ResponseFuture - Remove 3 dead class attributes (default_timeout, _profile_manager, _warned_timeout) that were never read or written on ResponseFuture - Add prepared_statement and _continuous_paging_state as class-level defaults (both None), skip __init__ assignment when parameter is None - Conditionalize _metrics and _host assignments: only set when non-None - Saves 4 STORE_ATTR operations per query on the common path (simple statements, no metrics, no host targeting, no continuous paging)
…te_response_future For prepared-statement workloads (the perf-critical case), BoundStatement is the most common query type reaching _create_response_future. Checking it before SimpleStatement saves one wasted isinstance() call per dispatch. Benchmark (80% BoundStatement, 15% SimpleStatement, 5% other): SimpleStatement first: 32.8 ns/dispatch BoundStatement first: 23.2 ns/dispatch Speedup: ~1.4-1.7x (~10-15 ns/dispatch saved)
Author
Follow-up commit: reorder isinstance chain in
|
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 free
to 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
ResponseFuture:default_timeout— belongs toSession, never used in RF_profile_manager— belongs toSession, never used in RF_warned_timeout— never read or written anywhere in the codebaseSTORE_ATTRoperations in__init__when parameters areNone(matching the class-level default):_metrics,prepared_statement,_host,_continuous_paging_stateprepared_statementand_continuous_paging_stateclass defaults (previously only set in__init__) to class level so the conditional skip works correctlyThread Safety
All skipped attributes have class-level
Nonedefaults and are only set during__init__or by the owning thread after construction. No lazy initialization of shared mutable state is introduced — the thread-safety invariants are preserved.Benchmark
ResponseFuture.__init__micro-benchmark,min()of 7 × 200k iterations:The common case (simple queries without metrics/prepared statements/host pinning) is the dominant path. The non-None case overhead is from the added
ifchecks, but these params are rarely all non-None simultaneously.Tests
All 645 unit tests pass (43 skipped), matching the origin/master baseline.