Skip to content

Cache one nodebuilder per emit resolver, make emit resolver emit context scoped - #64649

Open
Wesley Wigham (weswigham) wants to merge 11 commits into
microsoft:mainfrom
weswigham:fix-emitcontext-emitresolver-layers
Open

Wesley Wigham (weswigham) wants to merge 11 commits into
microsoft:mainfrom
weswigham:fix-emitcontext-emitresolver-layers

Conversation

@weswigham

Copy link
Copy Markdown
Member

Fixes #64625

Comment thread tsc/internal/checker/checker.go Outdated
Comment thread tsc/internal/checker/nodebuilderimpl.go Outdated

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.

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 5 Medium severity

Open (5)
What changed in this PR

This PR refactors emit resolver creation to be EmitContext-aware, updates related interfaces, and adjusts transformer/checker call sites to use the new API while sharing resolver link state via Checker.

Changes:

  • Update GetEmitResolver APIs to accept an EmitContext and remove EmitContext parameters from EmitResolver node-construction methods.
  • Centralize resolver link stores on Checker (EmitResolverLinks) and adjust EmitResolver to use them.
  • Update transformers, compiler emit host, language service, and tests to pass/create an EmitContext when requesting an emit resolver.
File Description
tsc/​internal/​transformers/​tstransforms/​importelision_test.go Updates tests to create an EmitContext and pass it into GetEmitResolver.
tsc/​internal/​transformers/​declarations/​util.go Switches helper functions from DeclarationEmitHost to printer.EmitResolver for flag checks.
tsc/​internal/​transformers/​declarations/​transform.go Routes flag checks and type construction through tx.resolver (context-bound) and updates resolver method calls.
tsc/​internal/​printer/​emitresolver.go Changes EmitResolver interface to no longer take EmitContext for node construction methods.
tsc/​internal/​printer/​emithost.go Updates EmitHost.GetEmitResolver signature to require an EmitContext.
tsc/​internal/​ls/​findallreferences.go Creates an EmitContext when requesting an emit resolver for visibility checks.
tsc/​internal/​compiler/​emitter.go Passes emitContext into host.GetEmitResolver.
tsc/​internal/​compiler/​emitHost.go Reworks emit host to produce emit resolvers via a function taking EmitContext.
tsc/​internal/​checker/​symbolaccessibility.go Switches to getDiagnosticsEmitResolver() for diagnostics-layer checks.
tsc/​internal/​checker/​nodebuilderimpl.go Switches to getDiagnosticsEmitResolver() for helper visibility/undefined checks.
tsc/​internal/​checker/​exports.go Switches to getDiagnosticsEmitResolver() for implicit undefined check.
tsc/​internal/​checker/​emitresolver.go Makes EmitResolver context-bound, caches a node builder per resolver, and moves link stores to Checker.
tsc/​internal/​checker/​checker.go Adds EmitResolverLinks, introduces GetEmitResolver(emitContext), and renames cached resolver accessor to getDiagnosticsEmitResolver().

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

Comment thread tsc/internal/checker/emitresolver.go
Comment thread tsc/internal/compiler/emitHost.go Outdated
Comment thread tsc/internal/compiler/emitHost.go Outdated
Comment thread tsc/internal/compiler/emitHost.go Outdated
Comment thread tsc/internal/transformers/declarations/transform.go
Comment thread tsc/internal/compiler/emitHost.go Outdated
import (
"context"
"sync"
"weak"

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.

Is this strictly required? This will lock us out of tinygo for sure...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What, the weak map? Yeah, we don't wanna leak emit contexts - since those retain node factories which in turn retain nodes (unless explicitly cleared). The cache has to be weak - or we have to break the contract of emit hosts managing emit contexts and shove emit host knowledge into the emit context just to manage a cache, which is.... bad.

You could always not cache but then every caller needs to be mindful of the lifetime, rather than letting the GC handle it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

GC's always been bad to us anyway, so I made ownership and freeing of emit contexts explicit now instead of having a cache - no more global cache, callers should either take one or create one for long duration tasks.

What this means in practice is that since we make one emitHost per thread per file, that emitHost now makes one EmitResolver during the course of emitting that file (shared for both declaration and js emit), which was made with one EmitContext used throughout the whole process. Since the same checker is used for multiple files, the resolver still needs to use the checker lock... but nothing else should need any threading stuff.

@jakebailey

Copy link
Copy Markdown
Member

Now this I like, though that double pointer is weird.

TypeScript Bot (@typescript-bot) test it

@typescript-automation

typescript-automation Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-automation

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-automation

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - native
Errors 41 41 ~ ~ ~ p=1.000 n=12
Symbols 115,276 115,276 ~ ~ ~ p=1.000 n=12
Types 79,613 79,613 ~ ~ ~ p=1.000 n=12
Memory Used 142,562k (± 0.44%) 139,492k (± 0.04%) -3,070k (- 2.15%) 139,315k 139,633k p=0.000 n=12
Memory Allocs 2,193,586 (± 0.01%) 2,190,770 (± 0.01%) -2,815 (- 0.13%) 2,190,429 2,191,328 p=0.000 n=12
Config Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Parse Time 0.040s (± 4.32%) 0.040s (± 3.22%) ~ 0.037s 0.043s p=0.499 n=12
Bind Time 0.012s (±11.52%) 0.013s (±17.49%) ~ 0.010s 0.018s p=0.919 n=12
Check Time 0.503s (± 0.83%) 0.494s (± 0.66%) -0.009s (- 1.84%) 0.487s 0.503s p=0.002 n=12
Emit Time 0.272s (± 2.94%) 0.271s (± 2.31%) ~ 0.253s 0.282s p=0.560 n=12
Total Time 0.835s (± 0.74%) 0.824s (± 0.83%) -0.011s (- 1.34%) 0.810s 0.844s p=0.027 n=12
angular-1 - native
Errors 3 3 ~ ~ ~ p=1.000 n=12
Symbols 845,762 (± 0.06%) 845,459 (± 0.06%) ~ 843,931 846,406 p=0.443 n=12
Types 248,123 (± 0.00%) 248,124 (± 0.00%) ~ 248,121 248,132 p=0.172 n=12
Memory Used 788,548k (± 0.05%) 784,468k (± 0.02%) -4,080k (- 0.52%) 784,001k 784,749k p=0.000 n=12
Memory Allocs 12,871,146 (± 0.10%) 12,845,426 (± 0.05%) -25,720 (- 0.20%) 12,836,882 12,869,908 p=0.000 n=12
Config Time 0.017s (± 1.07%) 0.016s (± 1.90%) 🟩-0.001s (- 3.45%) 0.016s 0.017s p=0.009 n=12
Parse Time 0.260s (± 1.96%) 0.255s (± 2.69%) ~ 0.240s 0.274s p=0.183 n=12
Bind Time 0.061s (± 8.82%) 0.062s (± 8.74%) ~ 0.057s 0.088s p=0.755 n=12
Check Time 0s 0s ~ ~ ~ p=1.000 n=12
Emit Time 1.589s (± 0.59%) 1.615s (± 1.43%) ~ 1.571s 1.684s p=0.071 n=12
Total Time 1.938s (± 0.61%) 1.965s (± 1.51%) ~ 1.917s 2.060s p=0.311 n=12
mui-docs - native
Errors 11,405 11,405 ~ ~ ~ p=1.000 n=12
Symbols 4,359,614 4,359,614 ~ ~ ~ p=1.000 n=12
Types 1,376,379 1,376,379 ~ ~ ~ p=1.000 n=12
Memory Used 2,955,951k (± 0.05%) 2,954,395k (± 0.04%) ~ 2,951,435k 2,957,715k p=0.242 n=12
Memory Allocs 32,838,929 (± 0.05%) 32,826,804 (± 0.05%) ~ 32,781,399 32,862,915 p=0.630 n=12
Config Time 0.017s (± 1.95%) 0.017s (± 1.99%) ~ 0.016s 0.017s p=1.000 n=12
Parse Time 0.497s (± 4.20%) 0.493s (± 3.74%) ~ 0.440s 0.543s p=0.766 n=12
Bind Time 0.002s 0.002s ~ ~ ~ p=1.000 n=12
Check Time 8.674s (± 0.62%) 8.641s (± 0.64%) ~ 8.512s 8.753s p=0.259 n=12
Emit Time 0.444s (± 7.09%) 0.528s (±14.24%) 🔻+0.084s (+18.89%) 0.417s 0.689s p=0.004 n=12
Total Time 10.381s (± 0.74%) 10.447s (± 0.37%) ~ 10.322s 10.520s p=0.183 n=12
strada-build-src - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,390,229 1,390,229 ~ ~ ~ p=1.000 n=12
Types 441,498 441,498 ~ ~ ~ p=1.000 n=12
Memory Used 1,666,734k (± 0.58%) 1,674,692k (± 0.90%) ~ 1,651,159k 1,717,637k p=0.932 n=12
Memory Allocs 91,966,181 (± 0.18%) 92,136,607 (± 0.15%) ~ 91,704,632 92,399,092 p=0.078 n=12
Config Time 0.004s (± 9.04%) 0.004s (±12.12%) ~ 0.002s 0.004s p=1.000 n=12
Parse Time 0.192s (± 3.82%) 0.193s (± 2.85%) ~ 0.181s 0.206s p=0.410 n=12
Bind Time 0.000s (±217.90%) 0.000s ~ ~ ~ p=1.000 n=12
Check Time 1.842s (± 0.41%) 1.816s (± 0.78%) -0.026s (- 1.43%) 1.771s 1.838s p=0.003 n=12
Emit Time 0.300s (± 3.62%) 0.293s (± 4.60%) ~ 0.246s 0.327s p=0.434 n=12
Total Time 23.697s (± 0.68%) 23.414s (± 0.57%) -0.283s (- 1.19%) 22.997s 23.809s p=0.020 n=12
strada-compiler - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 334,953 334,953 ~ ~ ~ p=1.000 n=12
Types 197,616 197,616 ~ ~ ~ p=1.000 n=12
Memory Used 313,311k (± 0.02%) 313,246k (± 0.04%) ~ 312,847k 313,500k p=0.630 n=12
Memory Allocs 4,623,069 (± 0.01%) 4,623,422 (± 0.01%) ~ 4,622,268 4,623,995 p=0.128 n=12
Config Time 0.001s 0.001s ~ ~ ~ p=1.000 n=12
Parse Time 0.113s (± 4.58%) 0.115s (± 5.01%) ~ 0.100s 0.131s p=0.580 n=12
Bind Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Check Time 1.065s (± 0.55%) 1.053s (± 0.44%) -0.012s (- 1.10%) 1.042s 1.066s p=0.004 n=12
Emit Time 0.123s (±12.61%) 0.132s (±12.44%) ~ 0.097s 0.158s p=0.523 n=12
Total Time 1.360s (± 0.85%) 1.353s (± 1.04%) ~ 1.323s 1.380s p=0.523 n=12
ts-pre-modules - native
Errors 87 87 ~ ~ ~ p=1.000 n=12
Symbols 303,834 303,834 ~ ~ ~ p=1.000 n=12
Types 181,663 181,663 ~ ~ ~ p=1.000 n=12
Memory Used 273,921k (± 0.01%) 273,954k (± 0.01%) ~ 273,859k 274,034k p=0.164 n=12
Memory Allocs 1,615,797 (± 0.01%) 1,616,020 (± 0.01%) ~ 1,615,475 1,616,727 p=0.114 n=12
Config Time 0.000s 0.000s (±217.90%) ~ 0.000s 0.001s p=1.000 n=12
Parse Time 0.097s (± 2.80%) 0.099s (± 5.61%) ~ 0.090s 0.113s p=0.944 n=12
Bind Time 0.036s (±12.98%) 0.035s (± 7.79%) ~ 0.030s 0.047s p=0.787 n=12
Check Time 0.830s (± 0.63%) 0.823s (± 0.56%) -0.007s (- 0.89%) 0.814s 0.833s p=0.043 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 0.979s (± 0.85%) 0.973s (± 0.76%) ~ 0.960s 0.996s p=0.259 n=12
vscode - native
Errors 380 380 ~ ~ ~ p=1.000 n=12
Symbols 10,709,444 10,709,444 ~ ~ ~ p=1.000 n=12
Types 3,547,731 3,547,731 ~ ~ ~ p=1.000 n=12
Memory Used 6,933,123k (± 0.02%) 6,932,406k (± 0.02%) ~ 6,930,041k 6,936,304k p=0.203 n=12
Memory Allocs 53,138,143 (± 0.02%) 53,205,842 (± 0.02%) +67,698 (+ 0.13%) 53,190,927 53,233,893 p=0.000 n=12
Config Time 0.073s (± 0.45%) 0.073s (± 0.42%) ~ 0.073s 0.074s p=0.680 n=12
Parse Time 1.782s (± 3.65%) 1.772s (± 3.86%) ~ 1.654s 1.954s p=0.799 n=12
Bind Time 0.575s (±17.50%) 0.627s (±18.04%) ~ 0.441s 0.887s p=0.326 n=12
Check Time 14.057s (± 1.07%) 13.975s (± 1.25%) ~ 13.606s 14.233s p=0.561 n=12
Emit Time 4.725s (±11.82%) 4.903s (±12.53%) ~ 4.014s 6.089s p=0.514 n=12
Total Time 21.297s (± 2.14%) 21.437s (± 2.29%) ~ 20.703s 22.362s p=0.514 n=12
webpack - native
Errors 589 589 ~ ~ ~ p=1.000 n=12
Symbols 1,301,301 1,301,301 ~ ~ ~ p=1.000 n=12
Types 615,528 615,528 ~ ~ ~ p=1.000 n=12
Memory Used 1,005,782k (± 0.02%) 1,005,769k (± 0.01%) ~ 1,005,501k 1,006,107k p=0.932 n=12
Memory Allocs 6,547,417 (± 0.02%) 6,547,308 (± 0.03%) ~ 6,544,081 6,554,808 p=0.478 n=12
Config Time 0.009s 0.009s ~ ~ ~ p=1.000 n=12
Parse Time 0.289s (± 4.46%) 0.285s (± 3.05%) ~ 0.262s 0.303s p=0.681 n=12
Bind Time 0.086s (±16.94%) 0.089s (±18.79%) ~ 0.064s 0.131s p=0.619 n=12
Check Time 1.929s (± 0.57%) 1.921s (± 0.68%) ~ 1.881s 1.946s p=0.401 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 2.349s (± 0.42%) 2.340s (± 0.73%) ~ 2.283s 2.375s p=0.541 n=12
xstate-main - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,054,027 1,054,027 ~ ~ ~ p=1.000 n=12
Types 387,065 387,065 ~ ~ ~ p=1.000 n=12
Memory Used 611,524k (± 0.03%) 611,359k (± 0.02%) ~ 611,084k 611,742k p=0.081 n=12
Memory Allocs 4,871,147 (± 0.08%) 4,867,469 (± 0.06%) ~ 4,860,096 4,877,048 p=0.128 n=12
Config Time 0.003s 0.003s ~ ~ ~ p=1.000 n=12
Parse Time 0.142s (± 4.05%) 0.142s (± 4.36%) ~ 0.130s 0.163s p=0.766 n=12
Bind Time 0.047s (±19.89%) 0.048s (±19.76%) ~ 0.033s 0.071s p=0.878 n=12
Check Time 1.209s (± 0.76%) 1.204s (± 0.51%) ~ 1.188s 1.218s p=0.433 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 1.409s (± 0.89%) 1.404s (± 0.76%) ~ 1.376s 1.426s p=0.560 n=12
System info unknown
Hosts
  • native
Scenarios
  • Compiler-Unions - native
  • angular-1 - native
  • mui-docs - native
  • strada-build-src - native
  • strada-compiler - native
  • ts-pre-modules - native
  • vscode - native
  • webpack - native
  • xstate-main - native
Benchmark Name Iterations
Current pr 12
Baseline baseline 12

Developer Information:

Download Benchmarks

@typescript-automation

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing baseline and pr:

Everything looks good!

This branch has not been deployed

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

Labels

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

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

Declaration emit re-walks a package.json exports map for every declaration (module specifier cache not shared across node builders)

3 participants