Skip to content

perf(python): climb the walk cursor for scope-directive and default checks - #2419

Open
Fieldnote-Echo wants to merge 2 commits into
DeusData:mainfrom
Fieldnote-Echo:perf/py-lexical-ancestor-kind
Open

Fieldnote-Echo wants to merge 2 commits into
DeusData:mainfrom
Fieldnote-Echo:perf/py-lexical-ancestor-kind

Conversation

@Fieldnote-Echo

Copy link
Copy Markdown

Fixes #2417

What does this PR do?

For nearly every Python identifier, usage extraction climbs toward the root with ts_node_parent, looking for a global/nonlocal ancestor (lexical_ancestor_kind, internal/cbm/extract_usages.c:1883) and for a parameter default (python_default_value_reference, :1782). Each ts_node_parent call descends from the root again, so an expression N terms deep costs O(N³). On error-free trees the climbs now follow the walk's occurrence cursor (O(1) per hop, as #2352 did for ReScript). For global/nonlocal one parent step is enough, because the grammar makes those names direct children of the statement. Trees with errors keep today's climb. The graph is byte-identical on 8 corpora, error-recovery trees included.

Linux x86_64, 32 logical CPUs, GCC 16 main this PR
Definitions pass, generated r = c0*x + …, N = 1000 / 2000 16.8 s / unfinished at 90 s 40 / 150 ms
Definitions pass, sympy 1.14 resolvent_lookup.py alone 8.7-8.8 s 53-54 ms
parallel_extract median, 823 / 2063 ordinary .py files 1.54 / 4.75 s 1.15 / 3.58 s

extract_python_deep_default_parent_climbs_are_linear counts root-descending parent steps at 200 and 400 terms: 0 here, 62,544 and 245,044 on main with the same counter wired in. Unmodified main passes it, because its climbs do not report to the counter. repro_python_scope_directives_bind_the_named_scope covers the global/nonlocal binding.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
    (run as make -f Makefile.cbm test-par)
  • Lint passes (make -f Makefile.cbm lint-ci)
    (lint-ci stops at cppcheck with the same findings as on main, none in files touched here; lint-format, lint-no-suppress and lint-memory-core pass)
  • New behavior is covered by a test (reproduce-first for bug fixes)

Prepared with AI assistance (exploration, reproduction, verification, implementation); the spec and design decisions are mine, and I reviewed and ran everything above.

…hecks

For nearly every Python identifier, lexical_ancestor_kind and
python_default_value_reference climbed toward the root with ts_node_parent,
which re-descends from the root on every step, so a deep expression was
cubic. In error-free trees, climb the walk's occurrence cursor instead and
take a single parent step for global/nonlocal names, which the grammar makes
direct children; trees with errors keep the original climb.

sympy's polys/numberfields/resolvent_lookup.py drops from about 8.7 s to
54 ms in the definitions pass, ordinary Python extraction is about 25%
faster, and the graph is byte-identical.

Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

This branch has not been deployed

No deployments
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.

perf(python): usage extraction climbs every ancestor per identifier, cubic in expression depth

1 participant