Repository navigation
fix(js): a name the calling function binds is its local - #2226
Merged
Merged
Conversation
A JS/TS parameter or plain var/let/const above the reference shadows a same-named function declared elsewhere in the file, whichever strategy found it; destructured call results, member calls and followed aliases are left alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 5, 2026
…— graph byte-identical (#2332) (#2344) Indexing CPython took about three times as long on 1.6.2 as at 290e03f (193 s vs 59 s wall, 8.3 GB vs 3.5 GB peak on a 16-thread Windows box). A CPU profile put ~290 s of the resolver workers' 412 s on two Python checks that first shipped in 1.6.2: - pythonExternalWrites (#2291) read every .py file once per module global its own module types. CPython has 2,375 Python files against the resolver's 1,000-entry content cache, so every read went to disk (184 s). - isPythonLocallyBound (#2198, #2219) re-stripped the calling file for every (function, name), and, asked once per same-named candidate by fitsPythonCallShape, re-derived the enclosing function each time: 3.55M calls for 53K distinct answers (104 s). Now: - A global's own module is read first; when its bindings already leave the type unknown (319 of 379 globals on CPython), no other module is read. - One pass per resolution indexes which Python files spell a name after a dot or a quote, and which write through .__dict__; the write scan visits only those files, in the same path order. Comment stripping only blanks text, so any write the scan accepts is spelled that way in the raw text; a non-ASCII name still reads every file. Index keys are flat copies (a regex capture would pin its whole file). - The repo files an import names are resolved once per mapping, in a WeakMap that lives as long as the resolver's import cache keeps it. - isPythonLocallyBound keeps the current file's stripped lines and per-name module-level answer, and answers a ref once. Verification: nodes, edges and unresolved_refs dumps of CPython and pretix are byte-identical to 6560052. Medians of 3 interleaved CPython runs on a loaded machine: 290e03f 100 s / 271 s CPU / 3.3 GB, 6560052 322 s / 801 s / 7.9 GB, this change 123 s / 361 s / 5.5 GB. In the CPU profile pythonExternalWrites drops 184 s -> 16 s and isPythonLocallyBound 104 s -> 13 s. Most of the remaining gap to 290e03f on CPython is JS checks running on its bundled minified d3 (matchDestructuredCallResult, #2334; jsFunctionLocalScope from #2226). __tests__/python-resolution-work.test.ts counts the work instead of timing it: reads of untouched files, strips of the calling file, and node lookups with 16 same-named candidates (4 / 2 / 8 with the fix, 19 / 18 / 128 on main). Each count fails when only its own part of the fix is reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 5, 2026
… summary time covers the whole run (#2334) (#2362) Indexing pretix took 2.2x as long since #2163 (issue #2334), and the bundled minified d3 was what #2344 left of CPython's 1.6.2 slowdown. Two JS/TS checks that first shipped in 1.6.2 redid whole-file work for every reference: - matchDestructuredCallResult (#2163) ran for every bare call in a file holding `const {` / `let {` / `var {`: per call it re-tested that pattern on the source, then stripped, blanked and brace-walked every line above the call and matched the binding regex over it. - jsFunctionLocalScope (#2226) re-stripped the calling function's lines above each reference. In a minified script every function's text runs to the end of its one line, so each strip was the whole file. CPU profiles, main -> this change: matchDestructuredCallResult 82.4 s -> 1.7 s and jsFunctionLocalScope 12.3 s -> 5.2 s on pretix; 35.4 s -> 0.37 s and 23.3 s -> 1.0 s on CPython's Lib/profiling + Doc/_static. Now: - A file's `const { … } = f(…)` bindings are read once per resolution: the blanked code, each binding's end, callee and keys, the block it sits in and where that block closes, and the spans the blanker read as regex literals. A call looks up its own name; only a call through a destructured name scans the code between that binding and itself for shadowing, and only once the callee is found to return the key (a `require(…)` destructuring, for one, needs no scan). - Blanking the text above a call on its own gives the file's blanked code up to the call: the stripper and blanker look past a character only for a comment opener's second character and for a regex literal's closing `/`. So the prefix's bindings are the file's that end at or before the call (a match never reads past its `(`, and one running past the call holds no `{`…`(` to start another), and its brace stack follows from where each block closes. For a call inside a span the file reads as a regex literal (a division, in minified code) the prefix is the code up to the `/` plus the short tail blanked on its own; where a binding statement could run across that `/` (inside its `{ … }` or `< … >`), and for out-of-range positions, the old per-call scan still runs. - jsFunctionLocalScope strips a function's lines once and cuts at the reference's line end; functions that start on the same line share the regex answer by (file, first line, reference line, name). - The new memos, and JS_FN_LOCAL_MEMO (never cleared before), drop with the other per-file memos in clearNameMatcherMemos. The summary line printed orchestrator.indexAll's durationMs, which covers extraction only, beside node and edge totals recomputed after resolution: the issue's 19.9 s run printed "in 1.7s". CodeGraph.indexAll and sync now report the whole run's wall time, which `codegraph init`, `index` and `sync`, the MCP auto-sync log and the telemetry duration bucket read. Verification: nodes, edges and unresolved_refs dumps keyed by file|kind|qualified_name|start_line are byte-identical to origin/main 8998697 (with #2357's new destructuring and Vue template refs) on all 24 runs: pretix, mealie and CPython's Lib/profiling + Doc/_static, 8 each; and to deac771 (#2358, #2360, #2361) on one more round of each. A differential harness ran old and new matchDestructuredCallResult on every call position of mealie (10,330) and the CPython assets (9,170, 742 inside misread regex literals) and on 16,000 random token-soup sources (1.33M positions), and old and new jsFunctionLocalScope on ~500K (function, line, name) triples: no difference; it reports one for each deliberate mutation of the new code that is not provably equivalent. Medians of 4 interleaved runs on a loaded 16-thread Windows box, process CPU / wall / peak RSS: pretix 160.2 s / 75.9 s / 2.47 GB -> 88.7 s / 43.5 s / 1.93 GB; CPython assets 75.1 s / 37.6 s / 1.23 GB -> 32.1 s / 20.4 s / 0.81 GB; mealie (no bundles, the control) 39.2 s -> 38.6 s CPU. __tests__/js-resolution-work.test.ts counts strips: a file with a destructured binding and 40 bare calls is stripped for the check once (41 on main), a 40-line function once (40 on main); each count fails when only its own half of the fix is reverted. full-pipeline.test.ts requires indexAll's and sync's durationMs to cover a linking pass delayed 250 ms past the orchestrator's time (on main: 218 < 468). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 7, 2026
…g on Node 22 (#2424) go-ethereum's `codegraph init` sat in "Resolving refs" for good on Node 22 (killed after 1 h 55 min and 9,203 CPU-seconds). It was not Go: two resolver pool workers each sat on one 500-ref chunk of graphql/internal/graphiql/graphiql.min.js, a 980 KB bundle on one line (18,388 refs, 2,234 functions). Whether a JS function binds a name itself (jsFunctionLocalScope, #2226) is read off its comment-stripped code with a parameter-list regex that starts with a `(?<!\b(?:if|while|for|switch|with)\s*)` lookbehind. Node 22's V8 compiles new regexes without optimization once a process has generated about a megabyte of regex code and holds 16 MB of executable memory, which a pool worker reaches after some 30,000 refs. Unoptimized, the lookbehind runs at every position and its `\s*` reads back through the whole run of blanks before it, so a run of n blanks costs n^2/2 steps. The comment stripper, which doesn't know regex literals, reads the `//` that closes `/Trident\//` as a comment and blanks the remaining 961,968 characters of the line. In a worker's state the regex took 36 ms over 16 K characters of that line, 5.1 s over 32 K and 54 s over 64 K, against 0.23 ms over 128 K in a fresh process. Compiling 20,000 throwaway regexes first reproduces it in a bare Node 22 process, and --no-regexp-optimization reproduces it anywhere. Node 24's V8, the runtime release bundles ship, kept optimizing after 60,000, so released installs finish: 1.6.2 indexes go-ethereum on its bundled Node 24 and stalls the same way on Node 22. The stall is as old as #2226. - A leading `(?=\()` keeps the lookbehind to where a parameter list opens. Every match starts with `(` anyway, so the regex matches exactly what it did. - The jsx-render pass split the file, sliced a function's lines and scanned them for tags once per function: 2,234 times over the bundle's line, 25 of the 28 s its linking passes took on Node 22 (9 of 13 on Node 24), for no edges. Functions that span the same lines now share one scan, and the file is split once. Graph: go-ethereum dumps (nodes, edges, unresolved refs, files) are byte-identical between main on Node 24 (main never finishes on Node 22) and this change on Node 22 and on Node 24. Main and this change also match byte for byte on bootstrap4, legend-state, takenote, mantis, excalidraw (482 jsx-render edges), qwik, gin, prometheus, etcd and hugo. Time, go-ethereum: on Node 22 main never finishes and this change takes 36 s; on Node 24 (three interleaved rounds, medians) 42.1 s -> 38.3 s wall, with resolution 17.7 s -> 12.7 s and the linking passes 6.5 s -> 1.5 s. Tests: js-local-binding-backtracking forces V8's unoptimized mode and indexes a function with a 60,000-character comment before a call (69 s on main, 1.4 s now); jsx-render-work counts the tag scans of 60 components on one line (60 on main, 1 now). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
What
A JS/TS name the calling function binds itself now means that local, not a same-named function declared elsewhere in the file. "Binds itself" means a parameter, or a plain
var/let/constabove the reference. This applies to calls, value references,function_refs andnew X(). It is checked on the final result inmatchReference, so fuzzy can't re-commit to the shadowed function. InmatchFunctionRefit narrows to declarations inside the binding function.Left alone:
const { t } = useI18n()/const { getLabel } = useProps(props). It re-binds what a call returns under the same name, and that is usually the same-named function. An early cut dropped excalidraw's 83t(…)calls.const indexName = this.dataSource.namingStrategy.indexName(…), read throughbareCallReceiver.const selected = useStore(s => s.reset); selected().if (openMarkerClose) {is not a parameter list. The return-type clause of the parameter pattern also stays on its line, so a ternary: data.slice()isn't read as a type.Why
lodash defines every helper inside
runInContext, and an IIFE there declaresfunction object() {}.objectis lodash's most common parameter name, sobaseHas(object, key),mixin(object, …)callingobject(this.__wrapped__)and 80+ others all reached that function. That was the repo's top references hub.A/B (wasm both arms)
object/iteratee/resultparams → the nested functionsnew Class(…)whereClassis the function's parameter;new Err(…)queryparameter → a test file'squery()isLeaf/forceUpdate/callbackparamspolygon/snapshot/canvasparamsthis.route(path)'spathparam →function path(loader) => loader?.(), alet AsyncLocalStoragelocalTests
__tests__/js-function-local-binding.test.ts. The parameter case fails on main. The second case pins that an unboundhandler(behindif (handler)) and aconst walk = …recursion still resolve.release-main-regressions(store selectors).🤖 Generated with Claude Code