Repository navigation
Use paged link stores for all symbol links - #64872
Open
Jake Bailey (jakebailey) wants to merge 1 commit into
Open
Jake Bailey (jakebailey) wants to merge 1 commit into
Jake Bailey (jakebailey) wants to merge 1 commit into
Conversation
Symbol links were stored in Go maps keyed by symbol pointer, so every
link lookup paid for a hash and probe. Map lookups for symbol links
were among the hottest leaf costs when checking large projects.
The paged store that already backs value symbol links indexes by the
dense symbol ID instead, which is a couple of loads. Using it for the
remaining symbol link kinds removes those map lookups, at the cost of
under 1% more heap for the page tables.
--checkers 4, n=8, default GC settings:
│ before │ after │
│ sec/op │ sec/op vs base │
Tsc/bluesky 2.826 ± 2% 2.799 ± 2% -0.97% (p=0.015 n=8)
Tsc/sentry 11.13 ± 3% 10.86 ± 2% ~ (p=0.083 n=8)
Tsc/t3code-server 13.97 ± 2% 13.89 ± 6% ~ (p=0.645 n=8)
geomean 7.603 7.502 -1.32%
│ before │ after │
│ rss-B │ rss-B vs base │
Tsc/bluesky 1.201Gi ± 2% 1.203Gi ± 2% ~ (p=0.878 n=8)
Tsc/sentry 3.647Gi ± 9% 3.535Gi ± 6% ~ (p=0.328 n=8)
Tsc/t3code-server 2.883Gi ± 4% 2.877Gi ± 3% ~ (p=0.574 n=8)
geomean 2.328Gi 2.304Gi -1.05%
│ before │ after │
│ heap-B │ heap-B vs base │
Tsc/bluesky 934.8Mi ± 0% 934.8Mi ± 0% ~ (p=0.798 n=8)
Tsc/sentry 2.583Gi ± 0% 2.603Gi ± 0% +0.78% (p=0.000 n=8)
Tsc/t3code-server 2.295Gi ± 0% 2.312Gi ± 0% +0.71% (p=0.000 n=8)
geomean 1.756Gi 1.765Gi +0.50%
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The migration consistently reuses the existing store implementation, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Replaces the checker’s remaining map-backed symbol links with existing ID-indexed paged stores to reduce lookup overhead.
Changes:
- Migrates all remaining symbol-link stores to
symbolLinkStore. - Renames the reverse-mapped store and updates its callers.
| File | Description |
|---|---|
| tsc/internal/checker/nodebuilderimpl.go | Updates reverse-mapped store references. |
| tsc/internal/checker/inference.go | Updates reverse-mapped link access. |
| tsc/internal/checker/emitsupport.go | Updates reverse-mapped lookup during emit. |
| tsc/internal/checker/checker.go | Converts remaining symbol-link stores to paged storage. |
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.
Symbol links were stored in Go maps keyed by symbol pointer, so every link lookup paid for a hash and probe. Map lookups for symbol links were among the hottest leaf costs when checking large projects.
The paged store that already backs value symbol links indexes by the dense symbol ID instead, which is a couple of loads. Using it for the remaining symbol link kinds removes those map lookups, at the cost of under 1% more heap for the page tables.
--checkers 4, n=8, default GC settings:
This is pretty small; want to check the real perf suite.