Repository navigation
Avoid allocations and redundant work when checking calls and object literals - #65040
Open
Jake Bailey (jakebailey) wants to merge 5 commits into
Open
Jake Bailey (jakebailey) wants to merge 5 commits into
Jake Bailey (jakebailey) wants to merge 5 commits into
Conversation
checkGrammarObjectLiteralExpression allocated a fresh map for every object literal it checked, even though most literals have zero or one property and so can never contain duplicate names, and the map is discarded as soon as the check returns. Only use a map when duplicates are possible (two or more properties outside of destructuring), and take it from a small per-checker free list so that repeated checks reuse the same backing storage. Nested object literals checked while computing property names take their own map from the list, so reentrancy is safe. Unusually large maps are not retained, since Go maps never shrink. Co-authored-by: Steven Chong <[email protected]>
checkExpressionForMutableLocation first asks isConstContext, which for literal-like expressions computes the expression's contextual type to see whether it is a const type parameter. When the answer is no, it then immediately computed the very same contextual type again to widen the literal. getContextualType walks up the tree and may consult resolved signatures, so doing it twice for every literal in a mutable location is wasted work. Let an internal variant of isConstContext hand back the contextual type it computed when that type alone decided the result, and use it for the widening step instead of recomputing it. When the answer came from an enclosing node instead, the type is recomputed as before. Co-authored-by: Steven Chong <[email protected]>
checkObjectLiteral eagerly allocated two symbol tables per literal: one for the properties of the literal type, and, under strictNullChecks, one collecting every property to detect properties that a later spread always overwrites. The latter is only ever consulted when the literal contains a spread, which most literals do not. Spreads also flush the current properties into the spread type and replace the table with a fresh one that frequently ends up unused, for instance when the spread is the last element. Allocate the overwrite-tracking table only when the literal contains a spread, and create the properties table on first use. Also skip mapType when remapping the final spread type if it is not a union, since there is only the type itself to inspect. Co-authored-by: Steven Chong <[email protected]>
Every call resolution allocated a fresh slice for its reordered candidate signatures, even though the list is only used while that resolution is in progress (unless signature help asks for it). Call resolution is one of the hottest paths in the checker, and most calls have a single candidate, for which the general reordering loop with its symbol lookups and slice insertions is unnecessary. Keep a small per-checker free list of candidate slices: a resolution takes one, and gives it back when it returns. Nested resolutions started while checking arguments take their own slice from the list, so they never share storage with an outer resolution in progress. Lists handed out through candidatesOutArray are never reused. Zero and one signatures skip the reordering logic entirely, as there is nothing to reorder. Co-authored-by: Steven Chong <[email protected]>
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The popped pool slot is not cleared, allowing the backing array to retain oversized maps and their property-name strings despite the retention limit.
1 open issue
What changed in this PR
This PR reduces allocations and repeated work in the native TypeScript checker when checking calls and object literals.
Changes:
- Reuse temporary candidate lists and duplicate-property maps.
- Create object-literal symbol tables lazily and bypass unnecessary type mapping.
- Reuse contextual types computed during const-context checks.
| File | Description |
|---|---|
tsc/internal/checker/grammarchecks.go |
Pools duplicate-property maps and skips allocation when duplicate checks are unnecessary. |
tsc/internal/checker/checker.go |
Reuses call-candidate storage and reduces object-literal allocation and contextual-type work. |
Review effort: Balanced
A popped entry stayed reachable from the free list's backing array until it was overwritten. If the borrowed value then grew too large to be returned to the pool, the stale slot kept it alive anyway, defeating the retention limit.
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.

Thanks Steven (@teamchong) for finding these!