Commit 515b4bb
authored
Optimize _add_global_declarations_for_language
Runtime improvement: the optimized version runs ~21% faster (242μs → 199μs). This improvement comes from avoiding repeated, expensive tree-sitter parses and short-circuiting obvious non-work cases.
What changed (key optimizations)
- Added small memoization layer for parser-backed functions:
- _cached_find_imports wraps analyzer.find_imports and caches results per (analyzer, source) using weakref.WeakKeyDictionary.
- _cached_find_module_level_declarations wraps analyzer.find_module_level_declarations similarly.
- The caches are keyed by the analyzer object and the source string so repeated requests for the same analyzer+source avoid re-parsing.
- WeakKeyDictionary ensures analyzers can still be garbage-collected (no leaking analyzers).
- Fast-path string check in _merge_imports:
- Before calling the parser, we check if "import" is present in new_source. If not, we skip parsing entirely, which is a very cheap test compared to a tree-sitter parse.
Why this speeds things up
- The profiler shows the dominant cost is tree-sitter parsing (find_imports / find_module_level_declarations ~0.9–1.0ms per parse in the samples). A single unnecessary parse dominates the function cost.
- Caching avoids duplicate parses when the same source is inspected multiple times in the same workflow (or across short-lived repeated calls using the same analyzer instance). For example, the function previously called parser-backed routines multiple times for the same source as it merged imports and built existing-name sets; now those repeated calls hit the cache instead of re-parsing.
- The "import" substring fast-path avoids the heavy parse in the common case where optimized code has no imports to merge at all. A simple string containment check is orders of magnitude cheaper than building a tree.
Behavior & dependency changes
- Behavior is unchanged functionally (same outputs for the same inputs). The only runtime-visible change is caching: results from analyzer.find_* calls may be reused for identical (analyzer, source) inputs.
- Memory: minor increase due to the caches, but weak references are used so analyzers won't be kept alive indefinitely.
Trade-offs / regressions
- Some microbench tests show tiny slowdowns (single-digit percent) in cases that always early-return or where inputs are unique (no cache hits). Those are expected: cache lookups and cache population introduce small overhead when there is no reuse. This is a reasonable trade-off because the dominant cases (where parsing would otherwise occur) are much faster.
- Overall the runtime metric (the primary acceptance reason) improved, and the trade-off is small memory and minimal lookup overhead.
Workloads that benefit most
- Cases with repeated analyses of the same source/analyzer pair (e.g., merging multiple declarations, repeated merges during a session) — caching avoids re-parsing.
- Common-case optimized inputs that don't contain imports — the fast-path avoids parsing completely and returns quickly.
- Hot paths that call these helpers multiple times per file will see the largest wins.
Evidence from profiles/tests
- The original profiler showed huge time spent in parser-backed calls; optimized profile shows those costs reduced or avoided where possible.
- Annotated tests show the large win in the JS parsing scenario (28% faster in the failing-analyzer case) and overall 21% speedup. Small slowdowns on early-return tests are minor and expected.
Summary
- Primary benefit: 21% runtime reduction by eliminating redundant tree-sitter parses and short-circuiting import-less inputs.
- Implementation uses safe, memory-friendly caches (weakref) and a cheap string fast-path to get the most performance where it matters, while keeping behavior intact. This makes the optimized code a practical win for real workloads that invoke these analyzers multiple times or need to merge imports frequently.1 parent 6ee3458 commit 515b4bb
1 file changed
Lines changed: 72 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
15 | 20 | | |
16 | 21 | | |
17 | 22 | | |
| |||
49 | 54 | | |
50 | 55 | | |
51 | 56 | | |
52 | | - | |
53 | | - | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
54 | 61 | | |
55 | 62 | | |
56 | 63 | | |
| |||
70 | 77 | | |
71 | 78 | | |
72 | 79 | | |
73 | | - | |
| 80 | + | |
74 | 81 | | |
75 | 82 | | |
76 | 83 | | |
| |||
85 | 92 | | |
86 | 93 | | |
87 | 94 | | |
88 | | - | |
| 95 | + | |
| 96 | + | |
89 | 97 | | |
90 | 98 | | |
91 | 99 | | |
| |||
227 | 235 | | |
228 | 236 | | |
229 | 237 | | |
230 | | - | |
231 | | - | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
232 | 244 | | |
233 | 245 | | |
234 | 246 | | |
| |||
291 | 303 | | |
292 | 304 | | |
293 | 305 | | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
0 commit comments