Repository navigation
perf(resolve): memoize ts_scope_scan — ambient-module lookup reparsed every .d.ts per import - #554
Merged
Merged
Conversation
ts_ambient_lookup walks every .d.ts file in the project for each out-of-repo import, and each ts_scope_scan reparsed the file through js_binding_tree. On a monorepo the ~400-file working set blew the 32-entry tree cache, so resolution reparsed every declaration file thousands of times (~69k parses/minute on supabase; the run never finished). Upstream memoizes its equivalent scan per file (TS_GLOBAL_SCOPES); do the same — each file is scanned once per resolver and shared thereafter. supabase index: never completed -> 46.9s, 327,838 edges.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
3 of 4 tasks
bompus
added a commit
that referenced
this pull request
Oct 27, 2026
Re-measured the README comparison on current builds after the resolver reconciliation: upstream 69af19a vs fork bc27fdb, Node 24.22.1 and Bun 1.4.4, same seven pinned corpora. The picture changed enough that the old claims would mislead: the fork now loses five of seven index races (supabase 2.3x, n8n 2.4x — the ambient declare-module port, with the #554 memo bounding the quadratic but not the residual per-ref x per-file scan), wins every memory comparison, syncs faster on six of seven, and wins the n8n MCP cells except busy memory (+18% on Node). Tally: 56 better / 5 no-clear-change / 15 worse of 76. render-charts.mjs now classifies by each comparison's own run-to-run spread (house-rules #321) instead of the fixed 3% band, drops killed/failed rows, and says "slower"/"more" when the fork regresses instead of "0.4x faster". adapt-flat-output.mjs stages the current runner's flat output for the renderer. Contaminated n8n MCP attempt disclosed in the doc.
bompus
added a commit
that referenced
this pull request
Oct 27, 2026
…easured (#556) * perf(resolve): port upstream ambient-module text scan + specifier index The ambient declare-module lookup (fork #548) ran ts_scope_scan, a tree-sitter parse, over every .d.ts once per out-of-repo import, then regex-tested each file's text per reference — ~1M parses on supabase before #554's memo and ~10.7k regex checks x ~2.4k refs after. Port upstream's ambient-modules.ts: a single text tokenizer per file (ts_tokenize) feeding scanAmbientModules/scanNamespaceBlocks state machines, memoized per file, plus a lazy specifier→files index (upstream's scanned.bySpecifier) so each import hits only candidate declaration files. Keeps the fork's export type { A as B } clause support and fixes three upstream-faithful port bugs found in review: export as namespace counts as a module marker, the namespace `global` flag means the namespace itself is a global name (members are global only via literal declare global blocks or script top level), and nested namespaces no longer leak into ambient member matching. Measured on 2026-10-29 against upstream 69af19a (same host, two runs per arm, median): supabase index 41.5 s -> 19.6 s on Node (upstream 18.2 s), n8n 178 s -> 68.4 s (upstream 74.9 s), discourse 22.2 s -> 14.4 s (upstream 19.6 s). supabase graph identical to the pre-tokenizer baseline (152,274 nodes / 327,838 edges); n8n loses 53 edges vs baseline (1,233,163 vs 1,233,216) from the corrected namespace-global admission. Validation: 304 focused resolver/scope tests pass; full vitest failure set identical to e9561f4 base (143, all pre-existing); kernel golden dumps unchanged; eval:precision holds every absent and present case on all nine pinned corpora (baselines committed). * docs(bench): re-measure index/sync after ambient-scan fix (c29349a) Index and sync cells re-measured 2026-10-29 on the same host with two runs per arm; MCP, CLI and explore cells carry the 2026-10-27 measurements at bc27fdb (a resolution-path change cannot move them). supabase 41.5 s -> 19.6 s Node (inside spread of upstream's 18.2 s), n8n 178 s -> 68.4 s (upstream 74.9 s — fork win), discourse 22.2 s -> 14.4 s (upstream 19.6 s). Losses drop from 15 to 10 of 76 comparisons; the residual index losses are pretix/Alamofire on Node and the Bun arm of supabase/n8n/pretix. n8n sync rises 4.18 -> 8.90 s (lazy ambient index is also paid during sync resolution) and stays ahead of upstream's 11.6 s. Card and heatmap regenerated; benchmark doc records the regression's closure and the three upstream-faithfulness fixes the port carried. * fix(resolve): tokenizer string escapes + close-brace offset (#556) Two boundary bugs in ts_tokenize found in review: - `\X` skipped two bytes unconditionally, so a multi-byte escaped char left `from` inside a UTF-8 sequence and a trailing backslash left `from = len + 1` — both panicking on the next slice — and a `\<newline>` continuation was skipped without counting the line. - `}` passed its post-increment offset, so block byte_end ran one char past the close brace, slicing one trailing char into block text and panicking if that char was multi-byte. Also corrects the README sync range (max is CPython 56%, not n8n) and splits the bench doc's build provenance by metric per the page header. 269 focused resolver tests pass on the rebuilt kernel. * test(eval): rebaseline precision at 2114130 Snapshots regenerated after the tokenizer fixes so codegraphSha names a build containing this PR rather than the e9561f4 base. All absent and present cases still hold on all nine corpora. * docs(readme): refresh Bun-vs-Node figures after re-measurement (#556)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ts_ambient_lookup(thetsOutsideTypeDeclarationport landed in the69af19acreconciliation) scans every.d.tsfile in the project for each out-of-repo import, and eachts_scope_scanre-parsed the file viajs_binding_tree.[tree-miss]instrumentation, allimportsrefs, 417 distinct.d.ts× ~2,440 reparses each).scanAmbientModulesis a tokenizer (never parses a tree) and its per-file scan is memoized (TS_GLOBAL_SCOPES). This ports the memo layer:ts_scope_memomakes each file's scope scan run once per resolver.#548(bisectedd0f288abfast →76937a38stalls); with this fix it finishes in 46.9 s (152,274 nodes / 327,838 edges).Test plan
npx vitest run __tests__/resolution.test.ts __tests__/kernel-golden-dumps.test.ts— 211 pass, golden dumps unchangednpx vitest run __tests__/ambient-module-imports.test.ts __tests__/alias-binding-resolution.test.ts— 65 pass