Skip to content

Speed up scope parsing, especially for deep scopes - #1428

Open
sirosen wants to merge 3 commits into
globus:mainfrom
sirosen:faster-scope-parsing
Open

Speed up scope parsing, especially for deep scopes#1428
sirosen wants to merge 3 commits into
globus:mainfrom
sirosen:faster-scope-parsing

Conversation

@sirosen

@sirosen sirosen commented Sep 8, 2026

Copy link
Copy Markdown
Member

There are three distinct optimizations here, applied in series:

  • Make scope parsing faster on deep scope strings
  • Remove unused scope graph 'nodes' set
  • Prefer 'reversed' over reversing slice

By way of explanation, the first optimization, which does some graph-coloring with an enum to search for back-edges.
This replaces the prior implementation, which made sets of "all ancestors" for each node, which results in a large number of new sets being built, especially for deeper scopes.

The micro-optimizations are very minor cleanup (the commits should be self-explanatory).

@codspeed-hq

codspeed-hq Bot commented Sep 8, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×3.9

⚡ 5 improved benchmarks
✅ 13 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
test_deep_scope_parsing[5000] 2,570.1 ms 398.8 ms ×6.4
test_deep_scope_parsing[4000] 1,606.2 ms 317.5 ms ×5.1
test_deep_scope_parsing[3000] 1,019.4 ms 236.9 ms ×4.3
test_deep_scope_parsing[2000] 478.2 ms 156.6 ms ×3.1
test_deep_scope_parsing[1000] 155.2 ms 76.5 ms ×2

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing sirosen:faster-scope-parsing (609c52b) with main (d32735a)

Open in CodSpeed

The graph intermediate representation tracked a set of nodes which is
never read. Removing it is a minor micro-optimization.
`reversed()` produces an iterator, rather than a brand new list, so it's
more lightweight. (This is a micro-optimization.)
@sirosen
sirosen force-pushed the faster-scope-parsing branch from b34723a to 609c52b Compare September 8, 2026 18:49
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.

1 participant