Paged link stores with fallback from array to map representation - #4329
Merged
Conversation
The checker keeps per-symbol and per-node side tables as core.LinkStore[K, V], a map[K]*V backed by an arena. The two hottest of these, valueSymbolLinks and symbolNodeLinks, dominate the mapaccess1_fast64 profile of a full check. Symbols and nodes already carry a lazily-assigned dense uint64 id, so these two stores can become paged arrays indexed by id. Lookup becomes a length check plus two slice indexes on the fast path, avoiding the hash, bucket probe, and pointer chase of a map lookup. This change adds core.IdLinkStore[V], a 256-entry-per-page paged array with the same Get/Has/TryGet surface as LinkStore, plus thin symbolLinkStore[V] and nodeLinkStore[V] wrappers in the checker that key by ast.GetSymbolId / ast.GetNodeId. Only the two hottest stores are converted; sparser stores stay map-backed to keep memory in check. VS Code src project, --noEmit, single-threaded: instructions:u (GOGC=off, min-of-3): 100.18G -> 94.22G (-5.9%) wall, n=14 interleaved median: 20.14s -> 17.28s (-14.2%) Memory used: 3584M -> 3429M (-4.3%) VS Code src project, --noEmit, parallel (default): wall, n=14 interleaved median: 6.16s -> 5.61s (-9.0%) Memory used: 4181M -> 4257M (+1.8%) Types/Symbols/Instantiations counters unchanged.
No functional change. The previous expression was already valid Go (make accepts any integer-typed length), but computing the delta in int avoids the inner uint64(len(...)) cast and reads more clearly.
Member
Author
|
@typescript-bot perf test this faster |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new paged, ID-indexed link-store implementation to reduce checker hot-path overhead, and migrates the two hottest checker link stores (symbolNodeLinks, valueSymbolLinks) onto it with different storage strategies (inline vs arena-backed) based on value size.
Changes:
- Add
core.PagedLinkStore[V]with dual page-table representations (array-backed vs map-backed) and fixed-size pages (*[256]V). - Add checker-specific wrappers (
nodeLinkStore,symbolArenaLinkStore) and switchChecker.symbolNodeLinks/Checker.valueSymbolLinksto use them. - Expose
ast.GetNextNodeId/ast.GetNextSymbolIdto choose the page-table strategy at checker initialization time.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/core/linkstore.go | Adds PagedLinkStore implementation and page sizing constants. |
| internal/checker/links.go | Introduces node/symbol link-store wrappers over PagedLinkStore. |
| internal/checker/checker.go | Migrates two checker link stores to the new paged implementations and initializes them in NewChecker. |
| internal/ast/utilities.go | Adds APIs to read the current global next node/symbol ID counters for strategy selection. |
|
@ahejlsberg, the perf run you requested failed. You can check the log here. |
Member
|
@typescript-bot perf test this faster |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
Latest commit hardens implementation for 32-bit architectures and automatically switches between page list and page map based on magnitude of key values. |
Member
Author
|
@typescript-bot perf test this faster |
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+45
to
+46
| pageMap map[uint64]*[pageSize]V // Page table for page indices below maxPageCount | ||
| pageList []*[pageSize]V // Page map for page indices above maxPageCount |
| return SymbolId(id) | ||
| } | ||
|
|
||
| func GetNextNodeId() NodeId { |
Member
|
LGTM but the two copilot comments I believe are accurate. |
jakebailey
approved these changes
Jul 22, 2026
ahejlsberg
enabled auto-merge
July 22, 2026 18:14
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 PR builds on #4226 with the following enhancements:
*[pageSize]V) instead of slices ([]V), reducing the size of the page table by 66%.SymbolNodeLinksare just a single pointer-sized field so they're stored directly in the pages of the link store.ValueSymbolLinksis a larger structure so they're stored as references into an arena.