Performance: cache ClassDef.ancestors() transitive walk - #3048
Performance: cache ClassDef.ancestors() transitive walk#3048Pierre-Sassoulas wants to merge 3 commits into
Conversation
4160de2 to
eba3439
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3048 +/- ##
==========================================
+ Coverage 93.67% 93.70% +0.03%
==========================================
Files 93 93
Lines 11645 11675 +30
==========================================
+ Hits 10908 10940 +32
+ Misses 737 735 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
eba3439 to
d7a22a7
Compare
Merging this PR will improve performance by 2.7%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | test_bench_endtoend_walk_infer_black |
35.7 s | 34.6 s | +3% |
| ⚡ | Simulation | test_bench_endtoend_walk_infer_flask |
23.1 s | 22.6 s | +2.4% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/cache-classdef-ancestors (6089a01) with main (da4a8cf)
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
2d391a6 to
2ef6d50
Compare
d7a22a7 to
2718c3e
Compare
…) bypass Add targeted regression coverage for the four perf commits in #3048 so the corner-case branches don't silently regress. scoped_nodes.py — ClassDef.ancestors() cache (TestAncestorsCaching): * cache hit reuses the materialized tuple across calls * recurs=False bypasses the cache entirely * cyclic class hierarchy (string.Template = A) unwinds via the _COMPUTING_ANCESTORS sentinel without infinite recursion * exception during the walk clears the sentinel so the cache is not poisoned (covers the except BaseException cleanup path) scoped_nodes.py — ClassDef._find_metaclass() cache (TestFindMetaclassCaching): * cached result reused on second no-context call * None result is cached so the MRO walk runs once * explicit context= argument bypasses the cache * re-entry through the _COMPUTING_METACLASS sentinel returns None (covers the cycle-break branch) brain_builtin_inference.py — single Call dispatcher (TestBuiltinDispatcher): * known builtin Name dispatches (bool, dict.fromkeys via Attribute) * unknown Name, non-dict Attribute, dynamic call target are skipped * re.Pattern = type(...) / re.Match = type(...) still excluded so brain_re keeps owning their inference * register_builtin_transform populates _BUILTIN_INFERENCE_FUNCS context.py — InferenceContext.clone() bypass (new tests/test_context.py): * path is an independent deep copy; mutations don't leak back * lookupname is reset to None on the clone * _nodes_inferred is shared (mutable counter preserved across the clone family — required for the max_inferred budget) * callcontext / boundnode / extra_context propagated by identity * constraints is shallow-copied * clone() bypasses __init__ (tracked via temporary patch) * end-to-end inference still resolves through a cloned context Closes the two PR #3048 coverage gaps reported by Codecov (scoped_nodes.py:2218-2220 and :2757).
2fb0a52 to
7e482f9
Compare
…) bypass Add targeted regression coverage for the four perf commits in #3048 so the corner-case branches don't silently regress. scoped_nodes.py — ClassDef.ancestors() cache (TestAncestorsCaching): * cache hit reuses the materialized tuple across calls * recurs=False bypasses the cache entirely * cyclic class hierarchy (string.Template = A) unwinds via the _COMPUTING_ANCESTORS sentinel without infinite recursion * exception during the walk clears the sentinel so the cache is not poisoned (covers the except BaseException cleanup path) scoped_nodes.py — ClassDef._find_metaclass() cache (TestFindMetaclassCaching): * cached result reused on second no-context call * None result is cached so the MRO walk runs once * explicit context= argument bypasses the cache * re-entry through the _COMPUTING_METACLASS sentinel returns None (covers the cycle-break branch) brain_builtin_inference.py — single Call dispatcher (TestBuiltinDispatcher): * known builtin Name dispatches (bool, dict.fromkeys via Attribute) * unknown Name, non-dict Attribute, dynamic call target are skipped * re.Pattern = type(...) / re.Match = type(...) still excluded so brain_re keeps owning their inference * register_builtin_transform populates _BUILTIN_INFERENCE_FUNCS context.py — InferenceContext.clone() bypass (new tests/test_context.py): * path is an independent deep copy; mutations don't leak back * lookupname is reset to None on the clone * _nodes_inferred is shared (mutable counter preserved across the clone family — required for the max_inferred budget) * callcontext / boundnode / extra_context propagated by identity * constraints is shallow-copied * clone() bypasses __init__ (tracked via temporary patch) * end-to-end inference still resolves through a cloned context Closes the two PR #3048 coverage gaps reported by Codecov (scoped_nodes.py:2218-2220 and :2757).
ea6b8c5 to
4f1ff78
Compare
4f1ff78 to
c009ecd
Compare
…) bypass Add targeted regression coverage for the four perf commits in #3048 so the corner-case branches don't silently regress. scoped_nodes.py — ClassDef.ancestors() cache (TestAncestorsCaching): * cache hit reuses the materialized tuple across calls * recurs=False bypasses the cache entirely * cyclic class hierarchy (string.Template = A) unwinds via the _COMPUTING_ANCESTORS sentinel without infinite recursion * exception during the walk clears the sentinel so the cache is not poisoned (covers the except BaseException cleanup path) scoped_nodes.py — ClassDef._find_metaclass() cache (TestFindMetaclassCaching): * cached result reused on second no-context call * None result is cached so the MRO walk runs once * explicit context= argument bypasses the cache * re-entry through the _COMPUTING_METACLASS sentinel returns None (covers the cycle-break branch) brain_builtin_inference.py — single Call dispatcher (TestBuiltinDispatcher): * known builtin Name dispatches (bool, dict.fromkeys via Attribute) * unknown Name, non-dict Attribute, dynamic call target are skipped * re.Pattern = type(...) / re.Match = type(...) still excluded so brain_re keeps owning their inference * register_builtin_transform populates _BUILTIN_INFERENCE_FUNCS context.py — InferenceContext.clone() bypass (new tests/test_context.py): * path is an independent deep copy; mutations don't leak back * lookupname is reset to None on the clone * _nodes_inferred is shared (mutable counter preserved across the clone family — required for the max_inferred budget) * callcontext / boundnode / extra_context propagated by identity * constraints is shallow-copied * clone() bypasses __init__ (tracked via temporary patch) * end-to-end inference still resolves through a cloned context Closes the two PR #3048 coverage gaps reported by Codecov (scoped_nodes.py:2218-2220 and :2757).
The clone() fast path no longer goes through __init__, which left the explicit nodes_inferred branch (context.py:56) uncovered — the one genuine coverage loss Codecov reports on #3048. Pin the __init__ contract for external callers: a passed cell is adopted by identity, the default is a fresh zeroed cell per context.
c009ecd to
8116a9e
Compare
…) bypass Add targeted regression coverage for the four perf commits in #3048 so the corner-case branches don't silently regress. scoped_nodes.py — ClassDef.ancestors() cache (TestAncestorsCaching): * cache hit reuses the materialized tuple across calls * recurs=False bypasses the cache entirely * cyclic class hierarchy (string.Template = A) unwinds via the _COMPUTING_ANCESTORS sentinel without infinite recursion * exception during the walk clears the sentinel so the cache is not poisoned (covers the except BaseException cleanup path) scoped_nodes.py — ClassDef._find_metaclass() cache (TestFindMetaclassCaching): * cached result reused on second no-context call * None result is cached so the MRO walk runs once * explicit context= argument bypasses the cache * re-entry through the _COMPUTING_METACLASS sentinel returns None (covers the cycle-break branch) brain_builtin_inference.py — single Call dispatcher (TestBuiltinDispatcher): * known builtin Name dispatches (bool, dict.fromkeys via Attribute) * unknown Name, non-dict Attribute, dynamic call target are skipped * re.Pattern = type(...) / re.Match = type(...) still excluded so brain_re keeps owning their inference * register_builtin_transform populates _BUILTIN_INFERENCE_FUNCS context.py — InferenceContext.clone() bypass (new tests/test_context.py): * path is an independent deep copy; mutations don't leak back * lookupname is reset to None on the clone * _nodes_inferred is shared (mutable counter preserved across the clone family — required for the max_inferred budget) * callcontext / boundnode / extra_context propagated by identity * constraints is shallow-copied * clone() bypasses __init__ (tracked via temporary patch) * end-to-end inference still resolves through a cloned context Closes the two PR #3048 coverage gaps reported by Codecov (scoped_nodes.py:2218-2220 and :2757).
The clone() fast path no longer goes through __init__, which left the explicit nodes_inferred branch (context.py:56) uncovered — the one genuine coverage loss Codecov reports on #3048. Pin the __init__ contract for external callers: a passed cell is adopted by identity, the default is a fresh zeroed cell per context.
8116a9e to
a064244
Compare
…) bypass Add targeted regression coverage for the four perf commits in #3048 so the corner-case branches don't silently regress. scoped_nodes.py — ClassDef.ancestors() cache (TestAncestorsCaching): * cache hit reuses the materialized tuple across calls * recurs=False bypasses the cache entirely * cyclic class hierarchy (string.Template = A) unwinds via the _COMPUTING_ANCESTORS sentinel without infinite recursion * exception during the walk clears the sentinel so the cache is not poisoned (covers the except BaseException cleanup path) scoped_nodes.py — ClassDef._find_metaclass() cache (TestFindMetaclassCaching): * cached result reused on second no-context call * None result is cached so the MRO walk runs once * explicit context= argument bypasses the cache * re-entry through the _COMPUTING_METACLASS sentinel returns None (covers the cycle-break branch) brain_builtin_inference.py — single Call dispatcher (TestBuiltinDispatcher): * known builtin Name dispatches (bool, dict.fromkeys via Attribute) * unknown Name, non-dict Attribute, dynamic call target are skipped * re.Pattern = type(...) / re.Match = type(...) still excluded so brain_re keeps owning their inference * register_builtin_transform populates _BUILTIN_INFERENCE_FUNCS context.py — InferenceContext.clone() bypass (new tests/test_context.py): * path is an independent deep copy; mutations don't leak back * lookupname is reset to None on the clone * _nodes_inferred is shared (mutable counter preserved across the clone family — required for the max_inferred budget) * callcontext / boundnode / extra_context propagated by identity * constraints is shallow-copied * clone() bypasses __init__ (tracked via temporary patch) * end-to-end inference still resolves through a cloned context Closes the two PR #3048 coverage gaps reported by Codecov (scoped_nodes.py:2218-2220 and :2757).
The clone() fast path no longer goes through __init__, which left the explicit nodes_inferred branch (context.py:56) uncovered — the one genuine coverage loss Codecov reports on #3048. Pin the __init__ contract for external callers: a passed cell is adopted by identity, the default is a fresh zeroed cell per context.
|
@DanielNoord I'd like to merge this before release, great performance improvements, confirmed in the original issue #1115 (comment), and it's been ready for a long time. |
DanielNoord
left a comment
There was a problem hiding this comment.
Would it be possible to split out the individual performance improvements into separate PRs? This seems to include 4.
With an upcoming long holiday weekend I don't have a lot of time for reviews. Context switching to a bigger PR is probably something I wouldn't get to before the end of next week. I can probably make some time for smaller PRs in between meetings tomorrow :)
The recursive walk in ``ancestors(recurs=True)`` re-resolved shared base classes on every call, amplifying cost on deep MRO chains. Cache the materialized tuple as a ``cached_property`` so each ClassDef pays for its ancestors once, and the cache dies with the instance when the manager drops the AST. ``context`` is intentionally not part of the key — the result is path-independent and the walk's own ``yielded`` set handles cycle prevention. ``TestAncestorsCaching`` covers the corner cases: a cache hit reuses the materialized tuple, ``recurs=False`` bypasses the cache entirely, a cyclic hierarchy (``string.Template = A``) unwinds through the ``_COMPUTING_ANCESTORS`` sentinel instead of recursing forever, and an exception mid-walk clears the sentinel so a transient failure cannot poison the cache. Measured on pandas/core/frame.py (interleaved A/B, n=4): baseline 21.34s ± 0.18 -> patched 20.48s ± 0.13 (-4.0%) Cache hit rate on the same run: 98% (66k hits / 1.2k misses on 1k distinct ClassDefs). Larger speedups expected on codebases with deeper MROs (SQLAlchemy/Pydantic-shaped projects). Closes #1115
a064244 to
70eb9d8
Compare
DanielNoord
left a comment
There was a problem hiding this comment.
Why do we go via __dict__ and not just set this as an attribute? Pehaps with weak links
Type of Changes
Description
Split out of the original four-optimization PR, as requested — this one now contains only the
ClassDef.ancestors()cache, in a single commit with its tests.The recursive walk in
ancestors(recurs=True)re-resolved shared base classes on every call, amplifying cost on deep MRO chains. Cache the materialized tuple on the instance so eachClassDefpays for its ancestors once, and the cache dies with the instance when the manager drops the AST.contextis intentionally not part of the key: the result is path-independent and the walk's ownyieldedset handles cycle prevention.TestAncestorsCachingcovers the corner cases: a cache hit reuses the materialized tuple,recurs=Falsebypasses the cache entirely, a cyclic hierarchy (string.Template = A) unwinds through the_COMPUTING_ANCESTORSsentinel instead of recursing forever, and an exception mid-walk clears the sentinel so a transient failure cannot poison the cache.Measured on pandas/core/frame.py (interleaved A/B, n=4):
Cache hit rate on the same run: 98% (66k hits / 1.2k misses on 1k distinct ClassDefs). Larger speedups expected on codebases with deeper MROs (SQLAlchemy/Pydantic-shaped projects).
The other three optimizations are now standalone PRs, each reviewable independently:
ClassDef._find_metaclass()per nodeCalldispatcher__init__inInferenceContext.clone()Note that CodSpeed's earlier +9.65% on this PR was the combined figure for all four; taken alone this change is the -4.0% above.
Closes #1115