Cache instantiation expression types early enough to prevent reentrancy during printback #59931

weswigham · 2024-09-10T17:50:44Z

…cy during error printback

weswigham · 2024-09-10T17:52:54Z

src/compiler/checker.ts

        let hasSomeApplicableSignature = false;
        let nonApplicableType: Type | undefined;
        const result = getInstantiatedType(exprType);
+        links.instantiationExpressionTypes.set(exprType.id, result);
        const errorType = hasSomeApplicableSignature ? nonApplicableType : exprType;
        if (errorType) {
            diagnostics.add(createDiagnosticForNodeArray(getSourceFileOfNode(node), typeArguments, Diagnostics.Type_0_has_no_signatures_for_which_the_type_argument_list_is_applicable, typeToString(errorType)));


The root cause of the stack overflow was basically that the typeToString on this line eventually recursively calls into getInstantiationExpressionType to make the version of the type that's a member of the mapped type being printed, which is identical, but re-triggers the error emit and then spins.

weswigham · 2024-09-10T17:53:19Z

@typescript-bot test it

typescript-bot · 2024-09-10T17:53:23Z

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

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

DanielRosenwasser · 2024-09-10T18:24:46Z

src/compiler/checker.ts

+        if (!links.instantiationExpressionTypes) {
+            links.instantiationExpressionTypes = new Map();
+        }
+        if (links.instantiationExpressionTypes.has(exprType.id)) {
+            return links.instantiationExpressionTypes.get(exprType.id)!;


Suggested change

if (!links.instantiationExpressionTypes) {

links.instantiationExpressionTypes = new Map();

}

if (links.instantiationExpressionTypes.has(exprType.id)) {

return links.instantiationExpressionTypes.get(exprType.id)!;

links.instantiationExpressionTypes ??= new Map();

const cachedType = links.instantiationExpressionTypes.get(exprType.id);

if (cachedType) {

return cachedType;

DanielRosenwasser · 2024-09-10T18:25:27Z

src/compiler/types.ts

@@ -6238,6 +6238,7 @@ export interface NodeLinks {
    potentialReflectCollisions?: Node[];
    potentialUnusedRenamedBindingElementsInTypes?: BindingElement[];
    externalHelpersModule?: Symbol;     // Resolved symbol for the external helpers module
+    instantiationExpressionTypes?: Map<number, Type>; // Cache of instantiation expression types for the node


Is there any reason you cache by IDs instead of by Type?

Historic convention and habit, mostly. I can swap it if you'd like it.

typescript-bot · 2024-09-10T18:33:06Z

@weswigham
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 - node (v18.15.0, x64)}
_Errors	₃₀	₃₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_62,153	_62,153	_~	_~	_~	_{p=1.000 n=6}
_Types	_50,242	_50,242	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{193,241k (± 0.74%)}	_{194,279k (± 0.96%)}	_~	_192,459k	_195,995k	_{p=0.810 n=6}
_{Parse Time}	_{1.30s (± 0.75%)}	_{1.29s (± 1.77%)}	_~	_1.25s	_1.31s	_{p=0.402 n=6}
_{Bind Time}	_0.71s	_{0.71s (± 0.57%)}	_~	_0.71s	_0.72s	_{p=0.405 n=6}
_{Check Time}	_{9.60s (± 0.39%)}	_{9.58s (± 0.71%)}	_~	_9.52s	_9.69s	_{p=0.374 n=6}
_{Emit Time}	_{2.74s (± 1.01%)}	_{2.74s (± 0.45%)}	_~	_2.72s	_2.75s	_{p=0.870 n=6}
_{Total Time}	_{14.34s (± 0.25%)}	_{14.32s (± 0.61%)}	_~	_14.24s	_14.44s	_{p=0.470 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₇	₇	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_945,753	_945,729	_{-24 (- 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_410,067	_410,043	_{-24 (- 0.01%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{1,222,743k (± 0.00%)}	_{1,222,738k (± 0.00%)}	_~	_1,222,684k	_1,222,784k	_{p=0.936 n=6}
_{Parse Time}	_{6.64s (± 0.74%)}	_{6.65s (± 0.88%)}	_~	_6.57s	_6.75s	_{p=1.000 n=6}
_{Bind Time}	_{1.86s (± 0.28%)}	_{1.86s (± 0.34%)}	_~	_1.85s	_1.87s	_{p=0.386 n=6}
_{Check Time}	_{31.18s (± 0.39%)}	_{31.11s (± 0.40%)}	_~	_30.87s	_31.21s	_{p=0.688 n=6}
_{Emit Time}	_{15.09s (± 0.47%)}	_{15.06s (± 0.56%)}	_~	_14.96s	_15.19s	_{p=0.377 n=6}
_{Total Time}	_{54.76s (± 0.22%)}	_{54.68s (± 0.22%)}	_~	_54.51s	_54.80s	_{p=0.423 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,515,550	_2,515,550	_~	_~	_~	_{p=1.000 n=6}
_Types	_933,023	_933,023	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,350,318k (± 0.00%)}	_{2,350,386k (± 0.00%)}	_~	_2,350,267k	_2,350,538k	_{p=0.298 n=6}
_{Parse Time}	_{11.13s (± 0.26%)}	_{11.10s (± 0.17%)}	_~	_11.08s	_11.13s	_{p=0.124 n=6}
_{Bind Time}	_{2.59s (± 0.40%)}	_{2.59s (± 0.68%)}	_~	_2.56s	_2.61s	_{p=0.402 n=6}
_{Check Time}	_{85.12s (± 0.36%)}	_{85.39s (± 0.24%)}	_~	_85.03s	_85.60s	_{p=0.170 n=6}
_{Emit Time}	_{0.33s (± 2.29%)}	_{0.33s (± 2.29%)}	_~	_0.32s	_0.34s	_{p=1.000 n=6}
_{Total Time}	_{99.17s (± 0.33%)}	_{99.41s (± 0.21%)}	_~	_99.05s	_99.62s	_{p=0.173 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,250,213	_1,250,227	_{+14 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,146	_265,146	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,402,315k (± 0.01%)}	_{2,402,883k (± 0.02%)}	_~	_2,402,197k	_2,403,373k	_{p=0.066 n=6}
_{Parse Time}	_{5.09s (± 0.57%)}	_{5.11s (± 0.85%)}	_~	_5.05s	_5.17s	_{p=0.575 n=6}
_{Bind Time}	_{1.91s (± 0.81%)}	_{1.90s (± 0.44%)}	_~	_1.89s	_1.91s	_{p=0.051 n=6}
_{Check Time}	_{34.83s (± 0.19%)}	_{34.82s (± 0.30%)}	_~	_34.71s	_35.00s	_{p=0.688 n=6}
_{Emit Time}	_{3.32s (± 1.22%)}	_{3.36s (± 4.75%)}	_~	_3.25s	_3.68s	_{p=0.872 n=6}
_{Total Time}	_{45.14s (± 0.23%)}	_{45.19s (± 0.62%)}	_~	_45.00s	_45.75s	_{p=0.689 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,250,213	_1,250,227	_{+14 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,146	_265,146	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,477,545k (± 0.05%)}	_{2,477,016k (± 0.04%)}	_~	_2,475,900k	_2,478,345k	_{p=0.471 n=6}
_{Parse Time}	_{6.35s (± 0.68%)}	_{6.35s (± 0.43%)}	_~	_6.31s	_6.39s	_{p=0.936 n=6}
_{Bind Time}	_{2.03s (± 0.70%)}	_{2.04s (± 0.50%)}	_~	_2.03s	_2.06s	_{p=0.141 n=6}
_{Check Time}	_{41.29s (± 0.39%)}	_{41.37s (± 0.48%)}	_~	_41.11s	_41.65s	_{p=0.378 n=6}
_{Emit Time}	_{4.04s (± 4.09%)}	_{4.12s (± 4.55%)}	_~	_3.98s	_4.43s	_{p=0.575 n=6}
_{Total Time}	_{53.74s (± 0.38%)}	_{53.91s (± 0.35%)}	_~	_53.66s	_54.17s	_{p=0.230 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_259,449	_259,452	_{+3 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_105,986	_105,986	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{433,792k (± 0.01%)}	_{433,771k (± 0.02%)}	_~	_433,692k	_433,894k	_{p=0.575 n=6}
_{Parse Time}	_{2.85s (± 0.99%)}	_{2.84s (± 0.79%)}	_~	_2.82s	_2.88s	_{p=1.000 n=6}
_{Bind Time}	_{1.07s (± 0.38%)}	_{1.06s (± 0.78%)}	_~	_1.05s	_1.07s	_{p=0.527 n=6}
_{Check Time}	_{15.40s (± 0.38%)}	_{15.40s (± 0.52%)}	_~	_15.31s	_15.53s	_{p=0.810 n=6}
_{Emit Time}	_{1.40s (± 1.23%)}	_{1.39s (± 1.15%)}	_~	_1.38s	_1.42s	_{p=0.559 n=6}
_{Total Time}	_{20.71s (± 0.25%)}	_{20.69s (± 0.39%)}	_~	_20.58s	_20.80s	_{p=1.000 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₆₈	₆₈	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_225,018	_225,018	_~	_~	_~	_{p=1.000 n=6}
_Types	_94,249	_94,249	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{370,272k (± 0.03%)}	_{370,263k (± 0.02%)}	_~	_370,189k	_370,346k	_{p=0.936 n=6}
_{Parse Time}	_{2.76s (± 0.77%)}	_{2.76s (± 0.51%)}	_~	_2.74s	_2.78s	_{p=0.935 n=6}
_{Bind Time}	_{1.57s (± 0.74%)}	_{1.58s (± 0.77%)}	_~	_1.56s	_1.59s	_{p=0.242 n=6}
_{Check Time}	_{15.72s (± 0.20%)}	_{15.76s (± 0.50%)}	_~	_15.70s	_15.91s	_{p=0.573 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{20.05s (± 0.20%)}	_{20.09s (± 0.37%)}	_~	_20.04s	_20.24s	_{p=0.468 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₁	₁	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_3,044,226	_3,044,161	_{-65 (- 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_1,052,704	_1,052,649	_{-55 (- 0.01%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{3,152,425k (± 0.00%)}	_{3,152,421k (± 0.00%)}	_~	_3,152,350k	_3,152,500k	_{p=0.810 n=6}
_{Parse Time}	_{13.82s (± 0.59%)}	_{13.83s (± 0.32%)}	_~	_13.76s	_13.90s	_{p=0.936 n=6}
_{Bind Time}	_{4.35s (± 0.45%)}	_{4.36s (± 0.12%)}	_~	_4.35s	_4.36s	_{p=0.549 n=6}
_{Check Time}	_{81.49s (± 0.61%)}	_{81.25s (± 0.32%)}	_~	_80.99s	_81.73s	_{p=0.378 n=6}
_{Emit Time}	_{22.15s (± 0.62%)}	_{22.09s (± 0.46%)}	_~	_21.94s	_22.24s	_{p=0.747 n=6}
_{Total Time}	_{121.81s (± 0.43%)}	_{121.53s (± 0.27%)}	_~	_121.28s	_122.16s	_{p=0.471 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_277,022	_277,022	_~	_~	_~	_{p=1.000 n=6}
_Types	_112,817	_112,817	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{426,812k (± 0.02%)}	_{426,749k (± 0.01%)}	_~	_426,681k	_426,828k	_{p=0.298 n=6}
_{Parse Time}	_{4.89s (± 0.21%)}	_{4.89s (± 0.47%)}	_~	_4.87s	_4.93s	_{p=0.743 n=6}
_{Bind Time}	_{2.13s (± 0.55%)}	_{2.13s (± 0.97%)}	_~	_2.12s	_2.17s	_{p=1.000 n=6}
_{Check Time}	_{21.78s (± 0.32%)}	_{21.73s (± 0.11%)}	_~	_21.71s	_21.76s	_{p=0.228 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{28.80s (± 0.21%)}	_{28.75s (± 0.17%)}	_~	_28.70s	_28.83s	_{p=0.228 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_539,317	_539,317	_~	_~	_~	_{p=1.000 n=6}
_Types	_178,997	_178,997	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{483,996k (± 0.02%)}	_{484,068k (± 0.04%)}	_~	_483,860k	_484,288k	_{p=0.574 n=6}
_{Parse Time}	_{4.27s (± 0.51%)}	_{4.25s (± 0.43%)}	_~	_4.23s	_4.28s	_{p=0.060 n=6}
_{Bind Time}	_{1.56s (± 0.48%)}	_{1.55s (± 1.13%)}	_~	_1.52s	_1.57s	_{p=0.437 n=6}
_{Check Time}	_{22.52s (± 0.47%)}	_{22.58s (± 0.23%)}	_~	_22.48s	_22.63s	_{p=0.377 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{28.35s (± 0.44%)}	_{28.38s (± 0.19%)}	_~	_28.28s	_28.44s	_{p=0.873 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Compiler-Unions - node (v18.15.0, x64)
angular-1 - node (v18.15.0, x64)
mui-docs - node (v18.15.0, x64)
self-build-src - node (v18.15.0, x64)
self-build-src-public-api - node (v18.15.0, x64)
self-compiler - node (v18.15.0, x64)
ts-pre-modules - node (v18.15.0, x64)
vscode - node (v18.15.0, x64)
webpack - node (v18.15.0, x64)
xstate-main - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-09-10T18:55:41Z

Hey @weswigham, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

typescript-bot · 2024-09-10T19:12:07Z

@weswigham Here are the results of running the user tests with tsc comparing main and refs/pull/59931/merge:

Everything looks good!

typescript-bot · 2024-09-10T20:27:14Z

@weswigham Here are the results of running the top 400 repos with tsc comparing main and refs/pull/59931/merge:

Everything looks good!

weswigham added 2 commits September 10, 2024 10:33

Cache instantiation expression types early enough to prevent reentran…

ac986ab

…cy during error printback

Build fix, new baselines

a9898b9

weswigham requested review from sandersn and andrewbranch September 10, 2024 17:50

typescript-bot assigned weswigham Sep 10, 2024

typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Sep 10, 2024

weswigham commented Sep 10, 2024

View reviewed changes

DanielRosenwasser reviewed Sep 10, 2024

View reviewed changes

sandersn approved these changes Sep 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache instantiation expression types early enough to prevent reentrancy during printback #59931

Cache instantiation expression types early enough to prevent reentrancy during printback #59931

weswigham commented Sep 10, 2024

weswigham Sep 10, 2024

weswigham commented Sep 10, 2024

typescript-bot commented Sep 10, 2024 •

edited

Loading

DanielRosenwasser Sep 10, 2024

DanielRosenwasser Sep 10, 2024

weswigham Sep 10, 2024

typescript-bot commented Sep 10, 2024

tsc

typescript-bot commented Sep 10, 2024

typescript-bot commented Sep 10, 2024

typescript-bot commented Sep 10, 2024

Cache instantiation expression types early enough to prevent reentrancy during printback #59931

Are you sure you want to change the base?

Cache instantiation expression types early enough to prevent reentrancy during printback #59931

Conversation

weswigham commented Sep 10, 2024

weswigham Sep 10, 2024

Choose a reason for hiding this comment

weswigham commented Sep 10, 2024

typescript-bot commented Sep 10, 2024 • edited Loading

DanielRosenwasser Sep 10, 2024

Choose a reason for hiding this comment

DanielRosenwasser Sep 10, 2024

Choose a reason for hiding this comment

weswigham Sep 10, 2024

Choose a reason for hiding this comment

typescript-bot commented Sep 10, 2024

tsc

typescript-bot commented Sep 10, 2024

typescript-bot commented Sep 10, 2024

typescript-bot commented Sep 10, 2024

typescript-bot commented Sep 10, 2024 •

edited

Loading