Repository navigation
Split link store lookups into inlineable fast paths - #65019
Jake Bailey (jakebailey) wants to merge 1 commit into
Conversation
LinkStore.Get and PagedLinkStore.Get are called on nearly every checker link access, and almost all of those calls hit an existing entry. Both methods also contained the code to create the entry (allocating the map or growing the page list, allocating a page, and allocating the value from the arena), which made them too large for the Go compiler to inline, so every hit paid for a full call. Move the creation logic into out-of-line getSlow methods so that Get only does the lookup and falls back to getSlow on a miss. The goal is for the hit path to be inlined at call sites: LinkStore.Get now fits within the default inlining budget, and the smaller PagedLinkStore.Get body is cheaper to inline at the hot call sites that PGO selects. Co-authored-by: Steven Chong <25894545+teamchong@users.noreply.github.com>
|
TypeScript Bot (@typescript-bot) perf test this faster |
There was a problem hiding this comment.
🟢 Approval recommended
The review reports no active issues with the link-store lookup and allocation refactor.
No open issues 🎉
What changed in this PR
This PR separates existing-entry lookups from allocation logic in the compiler’s link stores, making frequent checker link accesses more amenable to Go inlining.
Changes:
- Keeps
LinkStore.Getfocused on lookup, moving entry creation intogetSlow. - Uses
TryGetfor thePagedLinkStore.Getfast path and moves page growth and allocation intogetSlow.
| File | Description |
|---|---|
tsc/internal/core/linkstore.go |
Splits both link-store getters into compact lookup paths and separate creation paths. |
Review effort: Balanced
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Ryan Cavanaugh (RyanCavanaugh)
left a comment
There was a problem hiding this comment.
Reviewed at 159bd6a. The fast/slow split preserves map and paged-store lookup, allocation, and pointer identity behavior. Core tests and targeted compiler baselines exercising spreads, reverse-mapped properties, and parameter self references passed. No blocking issue found.
|
Hm, seems like virtually no difference. |
LinkStore.Get and PagedLinkStore.Get are called on nearly every checker link access, and almost all of those calls hit an existing entry. Both methods also contained the code to create the entry (allocating the map or growing the page list, allocating a page, and allocating the value from the arena), which made them too large for the Go compiler to inline, so every hit paid for a full call.
Move the creation logic into out-of-line getSlow methods so that Get only does the lookup and falls back to getSlow on a miss. The goal is for the hit path to be inlined at call sites: LinkStore.Get now fits within the default inlining budget, and the smaller PagedLinkStore.Get body is cheaper to inline at the hot call sites that PGO selects.
Thanks Steven (@teamchong) for finding this!