perf(resolution): walk the Python tree iteratively, not recursive yield-from - #3501
perf(resolution): walk the Python tree iteratively, not recursive yield-from#3501abhay-codes07 wants to merge 1 commit into
Conversation
… yield-from _walk_python_tree drove the symbol-resolution passes through a recursive generator: one suspended frame per tree level, every node re-propagated up the whole chain — ~25M frame resumptions on a 364-file corpus for ~2.8M nodes. An explicit stack yields the identical preorder (node identity and order verified) in ~30% less time, and no longer risks RecursionError on a deeply nested tree. Same rewrite _walk_js_tree already carries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Rewrites _walk_python_tree from a recursive yield from to an explicit-stack iteration that yields nodes in the identical preorder (children pushed reversed), eliminating the per-ancestor generator-frame overhead (~25M resumptions on a 364-file corpus) and removing the recursion-depth ceiling on deeply nested trees. Adds tests/test_walk_python_tree_iterative.py asserting the iterative walk emits the exact same node objects in the same order as the old recursive form, visits each node once, and handles a 2000-deep tree that would previously overflow.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1850 functions depend on the 146 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 585 callers, 44 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 35 more — each is listed as a finding
Verification — 1850 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 802 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
30 of 269 test file(s) selected (11%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_forwarding_review_findings.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_multilang.py— impacttests/test_package_json_subpath_imports.py— impacttests/test_pascal.py— impacttests/test_pascal_resolution.py— impacttests/test_phantom_external_import.py— impacttests/test_python_import_resolution.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_src_layout_import_resolution.py— impacttests/test_swift_computed_properties.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/test_unmapped_at_alias_resolution.py— impacttests/test_vue_extraction.py— impacttests/test_walk_python_tree_iterative.py— impact, changed-test
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
No difference found (not proven): No behavior difference found in \_walk\_python\_tree (not a proof).
The verifier ran both versions of \_walk\_python\_tree on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: concolic exploration (CrossHair). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 43 more finding(s) on lines outside this diff (see the check run).
What
_walk_python_tree— the preorder tree walk that drives every Python symbol-resolution pass — was a recursive generator:yield frombuilds one suspended generator frame per tree level and re-propagates every yielded node up the entire ancestor chain. On graphify's own 364-file corpus that is ~25M frame resumptions for ~2.8M actual nodes (4.4s of a sequential extract, per cProfile). This PR rewrites it as an explicit stack — the identical rewrite_walk_js_treeright above it already carries, with the same rationale in its comment.Why it's safe
[id(n) for n in iterative] == [id(n) for n in recursive]on real parsed trees (a class with methods, comprehensions, async/with, lambdas), plus the empty module.RecursionErroron a deeply nested tree (test parses 2000-deep nesting and walks it fine).cmpequal).Measured
Isolated micro-benchmark walking all 86 parsed trees in
graphify/(best of 5, same 594,438 nodes):v8)A 30% reduction on the walk itself; it feeds several resolution passes.
Tests
tests/test_walk_python_tree_iterative.py: node-identity/order equality with the recursive reference, visit-once, and the deep-nesting case. Full suite matches a freshv8(0.9.58) baseline.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q