Skip to content

perf(resolution): parse each Python file once across both resolution passes - #3502

Open
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/python-parse-memoization
Open

perf(resolution): parse each Python file once across both resolution passes#3502
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:perf/python-parse-memoization

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

What

The Python symbol-resolution facts pass (_collect_python_symbol_resolution_facts) and the cross-file import pass (_resolve_cross_file_imports) each parse the entire .py corpus, back to back, in the main process after the workers return. Every file was read from disk and tree-sitter-parsed twice for no reason — cProfile showed read_bytes firing ~5× per file and parse ~4× across the run.

_parse_python_tree now memoizes on (path, mtime_ns, size), and the cross-file pass — which previously built its own Parser and re-read each file — routes through it. The second pass reuses the first pass's tree.

Why it's safe

  • Both passes only read the tree (walk it, query it); a tree-sitter parse is immutable, so sharing one parse between them is behavior-preserving.
  • The cache key includes mtime and size, so a file edited between watch runs re-parses instead of replaying a stale tree (verified by test).
  • _parse_python_tree still returns None on any error exactly as before — callers already treat that as "skip this file".
  • The cross-file pass returned the parse Tree and read .root_node; the shared helper returns root_node directly, and the one downstream tree.root_node access was updated accordingly.
  • Graph is byte-identical before/after on the frozen 364-file self-corpus (sorted node + edge sets cmp equal).

Measured

Sequential extract of graphify's own 364-file Python corpus (best of 3, fresh cache dir each run):

best
v8 26.6s
this branch 23.8s

A ~11% reduction from eliminating one full parse (and disk read) of every Python file. Composes with the other resolution-pass perf work.

Tests

tests/test_python_parse_memoization.py: parse returns (source, root), missing file → None, one parse reused within a run, and an edited file re-parses rather than replaying. Full suite matches a fresh v8 (0.9.58) baseline.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q

…passes

The symbol-resolution facts pass and the cross-file import pass each parsed
the whole .py corpus, back to back, in the main process — every file read
and tree-sitter-parsed twice for nothing. _parse_python_tree now memoizes on
(path, mtime, size), so the second pass reuses the first pass's tree while a
file changed between watch runs still re-parses. On a 364-file self-corpus a
sequential extract drops from ~27s to ~24s, graph byte-identical.

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. No changes could be formally verified in this run.


Graphify review — findings

Memoizes _parse_python_tree on (path, mtime_ns, size) via _parse_python_tree_cached so the Python facts pass and the cross-file import pass share a single tree-sitter parse per file instead of reading and parsing the whole .py corpus twice; a file that changes between runs re-parses, and errors still return None (skip the file). _resolve_cross_file_imports now goes through the shared memo and keeps tree_sitter_python only as an availability check. Adds tests covering the source/root return, missing-file handling, within-run reuse, and re-parse on edit.

No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1848 functions depend on the 144 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 — 1848 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: 800 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_parse_memoization.py — impact, changed-test
  • 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

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

Could not verify: Could not verify \_parse\_python\_tree.

The verifier did not have enough to check \_parse\_python\_tree, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_resolve\_cross\_file\_imports.

The verifier did not have enough to check \_resolve\_cross\_file\_imports, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

· 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