Repository navigation
[XELP] Speed up scope validation when many scopes share files - #5
Merged
Merged
Conversation
detectVariableNamespaceCollisions walked every parameter and assignment up to the root of the tree to find an enclosing namespace, in every scope that includes the file. When the scope has no namespaces, getNamespace can never match, so return before walking.
ljunkie
force-pushed
the
rob/scope-validation-perf
branch
from
October 11, 2026 04:51
f46e3d3 to
13d2c25
Compare
Copilot stopped reviewing on behalf of
ljunkie due to an error
October 11, 2026 04:52
findFunctionCalls looks up the function scope of every call, and the lookup checked each function in the file in turn, so large files spent most of their parse time here. When every range is present, the top level functions are in order without overlapping, and each nested function sits inside its parent, the first scope containing a position is the top level one a binary search finds. Otherwise, such as after a parse error, the lookup checks every scope as before.
ljunkie
force-pushed
the
rob/scope-validation-perf
branch
from
October 11, 2026 05:02
13d2c25 to
dbd3f21
Compare
Copilot stopped reviewing on behalf of
ljunkie due to an error
October 11, 2026 05:03
iterateFileExpressions looked up the symbol table and the first name of every expression again for each scope that includes the file. The tables from an expression up to the file are the same in every scope; only the file table links on to the scope. So the symbol table, and whether the first name is declared in the file, are found once per file. A name declared in the file is always known, and in a scope with no namespaces or enums it has nothing further to check. Expressions inside a namespace are left to the full check, since namespace tables gain scope specific siblings.
validateComputedAAKeys walked the whole tree of every file again for each scope that includes it, looking for AAIndexedMemberExpression nodes. The nodes a file holds do not depend on the scope, so collect them once per file and check each against the scope. Most files have none, so the check costs nothing for them.
ljunkie
force-pushed
the
rob/scope-validation-perf
branch
from
October 11, 2026 05:08
dbd3f21 to
33c600d
Compare
Copilot stopped reviewing on behalf of
ljunkie due to an error
October 11, 2026 05:10
Comment on lines
859
to
+863
| if (!functionScopes) { | ||
| functionScopes = this.functionScopes; | ||
| } | ||
| //binary search the top level functions instead of checking every function in the file, when they allow it | ||
| const topLevelScopes = position && functionScopes === this._functionScopes ? this.getSortedTopLevelFunctionScopes() : undefined; |
Comment on lines
+186
to
195
| const symbolTable = expression.getSymbolTable(); | ||
| const enclosingNamespaceNameLower = expression.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript)?.toLowerCase(); | ||
| result.push({ | ||
| parts: parts, | ||
| expression: expression, | ||
| enclosingNamespaceNameLower: expression.findAncestor(isNamespaceStatement)?.getName(ParseMode.BrighterScript)?.toLowerCase() | ||
| symbolTable: symbolTable, | ||
| //namespace symbol tables gain scope specific siblings, so only resolve outside of namespaces | ||
| isDeclaredInFile: !enclosingNamespaceNameLower && this.isDeclaredInFile(file, symbolTable, parts[0].name?.text), | ||
| enclosingNamespaceNameLower: enclosingNamespaceNameLower | ||
| }); |
Comment on lines
+821
to
+825
| private getSortedTopLevelFunctionScopes() { | ||
| return this.cache.getOrAdd('sortedTopLevelFunctionScopes', () => this.findSortedTopLevelFunctionScopes(this.functionScopes)); | ||
| } | ||
|
|
||
| private findSortedTopLevelFunctionScopes(functionScopes: FunctionScope[]): FunctionScope[] | undefined { |
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.
Every component XML is its own scope, and BrighterScript repeated per-file work in each scope that includes a file. Plex's Roku client has 211 scopes sharing about 70k lines of fat scripts, so validation took about 20s on 0.73.6.
Validation of the Roku client's fat build drops from about 20s to about 5.5s. Diagnostics are identical to plain 0.73.6 on the fat and slim builds.
Timing
Validation of the Roku client's fat build, without bslint, three alternating runs:
The slim build drops from 13.6s to 5.0s.
How to test
npm test.bscagainst a Roku client fat build with and without this branch, and compare the time and the diagnostics.