Repository navigation
Inline storage for small symbol tables - #64996
Draft
Jake Bailey (jakebailey) wants to merge 2 commits into
Draft
Jake Bailey (jakebailey) wants to merge 2 commits into
Jake Bailey (jakebailey) wants to merge 2 commits into
Conversation
Small symbol tables need not allocate their entries separately. Across
4.71 million tables in the compiler suite, 36.3% peaked at four symbols
or fewer and 62.1% peaked at eight or fewer.
Four slots offer a better allocation/space tradeoff than eight: binding
allocations fall by 29-37%, while allocated bytes increase by at most
1.1%. Eight slots increased allocated bytes by 3-6%. Six slots cause
unfavorable slice growth for larger tables.
Binding benchmarks, with default GC settings:
│ built/symbol-table-bind-before.txt │ built/symbol-table-bind-four.txt │
│ sec/op │ sec/op vs base │
Bind/checker.ts-20 25.86m ± 14% 24.61m ± 1% -4.82% (p=0.004 n=6)
Bind/dom.generated.d.ts-20 11.21m ± 6% 10.68m ± 6% ~ (p=0.093 n=6)
Bind/Herebyfile.mjs-20 406.6µ ± 28% 326.9µ ± 19% ~ (p=0.240 n=6)
geomean 4.902m 4.413m -9.99%
│ built/symbol-table-bind-before.txt │ built/symbol-table-bind-four.txt │
│ B/op │ B/op vs base │
Bind/checker.ts-20 5.634Mi ± 0% 5.677Mi ± 0% +0.76% (p=0.002 n=6)
Bind/dom.generated.d.ts-20 3.363Mi ± 0% 3.400Mi ± 0% +1.09% (p=0.002 n=6)
Bind/Herebyfile.mjs-20 102.5Ki ± 0% 102.5Ki ± 0% +0.02% (p=0.002 n=6)
geomean 1.238Mi 1.246Mi +0.62%
│ built/symbol-table-bind-before.txt │ built/symbol-table-bind-four.txt │
│ allocs/op │ allocs/op vs base │
Bind/checker.ts-20 25.78k ± 0% 18.30k ± 0% -28.98% (p=0.002 n=6)
Bind/dom.generated.d.ts-20 24.45k ± 0% 15.44k ± 0% -36.86% (p=0.002 n=6)
Bind/Herebyfile.mjs-20 571.0 ± 0% 375.0 ± 0% -34.33% (p=0.002 n=6)
geomean 7.113k 4.733k -33.47%
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Focused tests are missing for the inline-storage boundaries, particularly symbol preservation when growing from four to five entries and independent backing storage when mutating small clones and their originals.
1 open issue
What changed in this PR
This PR reduces allocations in the native compiler’s symbol tables by storing up to four symbol pointers inline.
Changes:
- Use inline storage for small tables during construction and insertion.
- Clear inline references when entries grow beyond that storage.
- Give small clones their own inline backing storage.
| File | Description |
|---|---|
tsc/internal/ast/symbol.go |
Adds four-entry inline storage and updates table construction, growth, and cloning. |
Review effort: Balanced
Comment on lines
+292
to
+294
| if len(m.entries) == len(m.symbols)+1 { | ||
| // Once entries outgrow inline storage, don't retain the copied symbols. | ||
| clear(m.symbols[:]) |
Contributor
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
Jake Bailey (jakebailey)
added
the
Domain: Performance
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
Definitely fewer allocs but a bit unclear how much it matters. |
Member
|
Yeah, not sure this one is worth it. |
Member
|
Here's a thought - lots of symbol tables are just empty, or have fewer than 4 elements, or whatever. You are pre-allocating storage for small symbol tables, but ultimately the optimization here has a member pointing back to another member in the containing type just to avoid a second allocation and give you a bit more locality. Nobody ever needs to look at I wonder if we could work off of the stack and then decide on a variant of type SymbolMap struct {
entries []*Symbol
index []int32 // open addressing table of entry positions plus one (0 is empty), only past symbolTableLinearMax
}
type SymbolMap1Element {
SymbolMap
symbols [1]*Symbol
}
type SymbolMap2Elements {
SymbolMap
symbols [2]*Symbol
}
type SymbolMap3Elements {
SymbolMap
symbols [3]*Symbol
}
type SymbolMap4Elements {
SymbolMap
symbols [4]*Symbol
}When the |
Member
The thing is, the vast majority of symbol tables escape to the heap because they're attached to AST nodes or symbols. So you might as well put them there to begin with. |
Member
Author
|
Yeah, I just tried, and it doesn't seem to help (or makes things worse), because the compiler just isn't smart enough. |
Member
I meant more like using the stack as temporary storage and allocating the precise size after (similar to what I think the parser does). |
Ryan Cavanaugh (RyanCavanaugh)
approved these changes
Oct 30, 2026
Ryan Cavanaugh (RyanCavanaugh)
left a comment
Ryan Cavanaugh (RyanCavanaugh)
left a comment
Member
There was a problem hiding this comment.
The inline-storage growth and clone paths look correct. Existing AST tests and a focused overlay harness covering 42 capacity/size combinations pass, including independent mutations of clones.
Member
|
Ryan Cavanaugh (@RyanCavanaugh) Not really sure why we'd take this. It generally increases memory by 0.5% and has no positive effect on performance. |
Jake Bailey (jakebailey)
marked this pull request as draft
October 30, 2026 14:44
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.
This is something I brought up in review; inline some space for small tables.
I thought 1 would be right, but doing some stats on our tests, 4 appeared to be the magic number.