You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Avoid unnecessary work and allocations when instantiating type - #65035
Many types passed to instantiation cannot contain type variables, and
whether they can is already cached in their object flags. Check that
cached bit inline at the top of instantiateTypeWithAlias so the common
"nothing to do" case doesn't go through an indirect call.
getPermissiveInstantiation and getRestrictiveInstantiation previously
did a map lookup and one or two map inserts for every non-primitive
type, even though instantiating a variable-free type always yields the
type itself. Return such types directly instead. The restrictive
self-entry is only consulted for conditional types, which are never
variable-free, so skipping it is unobservable.
instantiateSymbol unconditionally fetched the symbol's value links,
allocating them for symbols whose types were never resolved just to
find them empty. Only look the links up, and check the cheap
this-only mapper case first.
Co-authored-by: Steven Chong <[email protected]>
instantiateTypeAlias allocated a new TypeAlias on every call, even
though instantiateTypes already returns the original argument slice
when no argument changes. This happens frequently, e.g. for aliases
without type arguments or whose arguments are unaffected by the
mapper, and the result is passed along on most object, union and
indexed access instantiations.
Return the existing alias in that case, extending the copy-on-change
behavior of list instantiation to the alias itself. Aliases are never
mutated after being attached to a type, and are compared by content
rather than identity, so sharing them is safe.
Co-authored-by: Steven Chong <[email protected]>
When instantiating an anonymous, mapped or deferred object type, and
when instantiating a type reference, we computed the new type
arguments into a freshly allocated slice, then used them to look up an
existing instantiation. Most of these lookups hit, at which point the
slice was simply discarded.
Compute the type arguments into a stack buffer instead, look up the
cache with it, and only copy the arguments to the heap when a new
instantiation is actually created. For type references that need no
tuple normalization, the cache key computed for the lookup is reused
when creating the reference.
Co-authored-by: Steven Chong <[email protected]>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unconditional cloning of already-owned type-argument slices introduces redundant allocations and copies on cache misses in both type-reference and anonymous-type instantiation paths.
When more type arguments are instantiated than fit in the stack
buffer, the list is already freshly allocated and owned by the
caller. Cloning it again before creating a new instantiation added a
second allocation and copy on every cache miss, making these cases
more expensive than before the buffer was introduced.
Only clone arguments that were written into the buffer, and pass
heap-allocated lists through as-is.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Temporary argument buffers still escape to the heap in both instantiation paths, causing avoidable allocations for unchanged arguments and cache hits, plus extra allocations on small-argument cache misses.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Prevent temporary buffer escape and excess allocations
tsc/internal/checker/checker.go:22991
The conditional clone leaves an escape path from buf to newTypeReferenceInstantiation, so the temporary buffer is heap-allocated even when the arguments are unchanged or the cache hits. The same issue occurs in getObjectTypeInstantiation at line 23073, where small-argument cache misses also allocate the cloned slice in addition to the buffer. Separate temporary storage from retained argument storage so neither buffer can escape, then verify allocation counts for unchanged arguments, cache hits, and cache misses.
Escape analysis does not consider control flow. Because the variable
that could point into the stack buffer was also handed to code that
retains it, the buffer was moved to the heap on every call, including
the unchanged and cache-hit paths that were meant to be allocation
free.
Keep retained arguments in a separate variable that is only assigned
from a fresh allocation or a copy, so the buffer never flows into
retained storage and can stay on the stack.
The reason will be displayed to describe this comment to others. Learn more.
🟢 Approval recommended
There are no active review issues blocking the type-instantiation optimizations.
No open issues 🎉
Review effort: Balanced
This branch has not been deployed
No deployments
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
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!