Skip to content

Avoid cloning cached declaration types after truncation - #63969

Open
Butros J. G. Groot (butros10games) wants to merge 3 commits into
microsoft:mainfrom
butros10games:codex/declaration-emit-truncation
Open

Avoid cloning cached declaration types after truncation#63969
Butros J. G. Groot (butros10games) wants to merge 3 commits into
microsoft:mainfrom
butros10games:codex/declaration-emit-truncation

Conversation

@butros10games

@butros10games Butros J. G. Groot (butros10games) commented Aug 23, 2026

Copy link
Copy Markdown

Fixes #63966

Context

When declaration emit serializes an inferred type, cached type nodes are
deep-cloned when they are reused. The cache avoids recomputing each type, but
every cache hit still materializes a fresh copy of the cached AST subtree.

For deeply shared structural types, those copies can grow exponentially even
after declaration serialization has exceeded its hard limit and will report
TS7056.

This PR

Type serialization now checks the existing node-builder truncation state before
visiting a type. Once the length limit has been reached, it uses the existing
elided-information placeholder instead of building or cloning another subtree.

Cache hits reached before truncation still replay tracked symbols and add their
cached length. If a cached node activates truncation, that node is cloned with
its existing inner elision; subsequent type visits return the placeholder
immediately.

The check lives in the shared type visitor rather than a declaration-specific
mode, matching existing node-builder truncation semantics. Existing compiler
and quick-info expectations are updated where the earlier check changes the
location or shape of the terminal placeholder.

A compiler test covers the issue reproduction without emitting JavaScript or
generating unnecessary type and symbol baselines.

Performance

Measured on the issue reproduction using native tsc --declaration:

Metric main This PR Change
Wall time 12.32 s 1.47 s -88.1%
Peak RSS 2,439,644 KiB 375,288 KiB -84.6%

Both runs report TS7056, emit identical JavaScript, and write no declaration
file.

Validation

  • npx hereby test:all
  • npx hereby lint
  • npx hereby check:format
  • TypeScript API build and tests
  • VS Code extension build and tests
  • Go module tidiness checks

This PR was developed with assistance from OpenAI Codex. I reviewed the final
diff and validation results.

Copilot AI balanced review requested due to automatic review settings August 23, 2026 15:32
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Aug 23, 2026
@typescript-automation typescript-automation Bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 23, 2026
@butros10games

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes declaration emit for oversized inferred types by eliding cached subtrees after truncation, preventing excessive cloning and memory use.

Changes:

  • Adds an internal node-builder truncation flag for declaration emit.
  • Replaces oversized cached subtrees with the existing elision placeholder.
  • Adds a compiler regression test and diagnostic baseline.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tsc/internal/checker/nodebuilderimpl.go Elides cached types after the serialization limit.
tsc/internal/nodebuilder/types.go Defines the new internal builder flag.
tsc/internal/transformers/declarations/transform.go Enables the optimization for declaration emit.
tsc/testdata/tests/cases/compiler/declarationEmitOversizedCachedType.ts Adds the oversized shared-type reproduction.
tsc/testdata/baselines/reference/compiler/declarationEmitOversizedCachedType.errors.txt Records the expected TS7056 diagnostic.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread tsc/internal/checker/nodebuilderimpl.go Outdated
b.ctx.truncating = true
}
b.ctx.approximateLength += cachedResult.addedLength
if b.ctx.internalFlags&nodebuilder.InternalFlagsStopBuildingAfterTruncation != 0 && b.checkTruncationLength() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't even think this flag is required. If we're truncating, we're truncating, no?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye, I agree - if we're truncating we emit ... instead of doing more work - this should just be a new place we check-trunating-and-emit ....

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve now committed the removal of the flag.

I initially had it without the flag locally, but started overthinking it. I also updated the two existing type baselines where cached subtrees are now elided after truncation.

Thanks for the review!

@github-project-automation github-project-automation Bot moved this from Not started to Needs merge in PR Backlog Aug 24, 2026

@weswigham Wesley Wigham (weswigham) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, hold up - I think we wanna move the truncation check up the function - so move the

if b.checkTruncationLength() {
	return b.createElidedInformationPlaceholder()
}

up to the start of the function. Reason being that if we load a cached node that has cached truncation, we want to continue truncating subsequent vsitAndTransformType calls, not blindly replace the result that already has truncation set (since it should already have inner truncation).

@github-project-automation github-project-automation Bot moved this from Needs merge to Waiting on author in PR Backlog Aug 24, 2026
@butros10games

Butros J. G. Groot (butros10games) commented Aug 24, 2026

Copy link
Copy Markdown
Author

Actually, hold up - I think we wanna move the truncation check up the function - so move the

if b.checkTruncationLength() {
	return b.createElidedInformationPlaceholder()
}

up to the start of the function. Reason being that if we load a cached node that has cached truncation, we want to continue truncating after it, not blindly replace it (since it should already have inner truncation).

Good catch, I’ve moved the truncation check to the start of visitAndTransformType. Cached nodes that activate truncation now keep their existing shortened output, while subsequent type visits stop immediately. I also updated the affected test expectations, and the full test suite passes. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Waiting on author

Development

Successfully merging this pull request may close these issues.

Performance regression for declaration emit of an oversized inferred type

4 participants