Skip to content

perf(resolution): walk the Python tree iteratively, not recursive yield-from - #3501

Open
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/iterative-walk
Open

perf(resolution): walk the Python tree iteratively, not recursive yield-from#3501
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/iterative-walk

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

What

_walk_python_tree — the preorder tree walk that drives every Python symbol-resolution pass — was a recursive generator:

def _walk_python_tree(node):
    yield node
    for child in node.children:
        yield from _walk_python_tree(child)

yield from builds 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_tree right above it already carries, with the same rationale in its comment.

Why it's safe

  • Yields the same node objects in the same preorder — children pushed reversed so the first child pops first. A test asserts [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.
  • As a bonus it no longer risks RecursionError on a deeply nested tree (test parses 2000-deep nesting and walks it fine).
  • Graph is byte-identical before/after on the frozen self-corpus (sorted node + edge sets cmp equal).

Measured

Isolated micro-benchmark walking all 86 parsed trees in graphify/ (best of 5, same 594,438 nodes):

best
recursive (v8) 135ms
iterative 95ms

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 fresh v8 (0.9.58) baseline.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q

… 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
Copilot AI lite review requested due to automatic review settings September 11, 2026 18:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 — impact
  • tests/test_build.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_extract.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_js_exported_scalar_bindings.py — impact
  • tests/test_languages.py — impact
  • tests/test_multilang.py — impact
  • tests/test_package_json_subpath_imports.py — impact
  • tests/test_pascal.py — impact
  • tests/test_pascal_resolution.py — impact
  • tests/test_phantom_external_import.py — impact
  • tests/test_python_import_resolution.py — impact
  • tests/test_python_underscore_resolution.py — impact
  • tests/test_rationale.py — impact
  • tests/test_ruby_resolution.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_src_layout_import_resolution.py — impact
  • tests/test_swift_computed_properties.py — impact
  • tests/test_ts_new_expression_calls.py — impact
  • tests/test_typescript_module_extensions.py — impact
  • tests/test_unmapped_at_alias_resolution.py — impact
  • tests/test_vue_extraction.py — impact
  • tests/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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants