Repository navigation
Speed up node lookup by position, import scanning, and node allocation - #65044
Open
Jake Bailey (jakebailey) wants to merge 4 commits into
Open
Jake Bailey (jakebailey) wants to merge 4 commits into
Jake Bailey (jakebailey) wants to merge 4 commits into
Conversation
GetNodeAtPosition descends from the source file by linearly scanning each node's children. At the top level that scan visits every statement in the file, and ForEachDynamicImportOrRequireCall calls GetNodeAtPosition for every textual "import"/"require" match, so files with many statements and many imports did quadratic work. In non-JS files the top-level statements are in source order and do not overlap, so the first statement whose end is past the position can be found by binary search; it is the only candidate that can contain the position. JS files keep the linear scan because reparsed JSDoc statements (@typedef, @import, ...) are inserted ahead of the statement that hosts them and their ranges nest within it. Co-authored-by: Steven Chong <[email protected]>
ForEachDynamicImportOrRequireCall scanned the source text with strings.IndexAny(text, "ir"), which walks the text byte by byte and stops at every 'i' and 'r' (both very common letters) only to compare and usually reject the keyword there. Search for each keyword with strings.Index instead, which uses the vectorized byte search, and remember the next occurrence of each keyword so it is only searched for again once the scan has passed it. Matches are produced in exactly the same order as before. Co-authored-by: Steven Chong <[email protected]>
The node factory already allocates the most common node kinds from per-factory arenas, which batches many small heap allocations into a few larger ones and keeps nodes of the same kind close together in memory. A number of other kinds that are created in large numbers when parsing real code and declaration files (imports, object and array literals, arrow functions, methods and properties, binding patterns, template literals, and several type nodes) were still allocated one at a time. Mark 28 more node kinds as arena-allocated so they take the same path. Co-authored-by: Steven Chong <[email protected]>
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the AST lookup, import scanning, and node allocation changes.
No open issues 🎉
1 file not reviewed
tsc/internal/ast/ast_generated.go: Generated file
What changed in this PR
This PR targets performance in the native TypeScript compiler by optimizing AST node lookup, import scanning, and node allocation.
Changes:
- Use binary search for top-level node lookup in non-JavaScript files, preserving linear traversal for JavaScript.
- Cache upcoming
importandrequirematches to avoid repeated searches. - Expand arena allocation to more AST node types through schema and generated factory updates.
| File | Description |
|---|---|
tsc/internal/ast/utilities.go |
Optimizes position-based node lookup and import/require scanning. |
tsc/internal/ast/ast_generated.go |
Adds arenas and updates generated constructors to use them. |
tools/scripts/tsc/ast.json |
Enables arena allocation for additional AST node types. |
Review effort: Balanced
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This branch has not been deployed
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.
Thanks Steven (@teamchong) for finding these!