Fixed cache key computation for tuple target types with partially named members #55695

Andarist · 2023-09-09T20:33:00Z

…ed members

Andarist · 2023-09-09T20:33:58Z

src/compiler/checker.ts

@@ -16392,10 +16392,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
            // [...X[]] is equivalent to just X[]
            return readonly ? globalReadonlyArrayType : globalArrayType;
        }
-        const memberIds = mapDefined(namedMemberDeclarations, node => node ? getNodeId(node) : undefined);


this would previously compute the same cache key segment for [undefined, labelNode] and [labelNode, undefined]

jakebailey · 2023-09-09T21:28:17Z

src/compiler/checker.ts

        const key = map(elementFlags, f => f & ElementFlags.Required ? "#" : f & ElementFlags.Optional ? "?" : f & ElementFlags.Rest ? "." : "*").join() +
            (readonly ? "R" : "") +
-            (memberIds.length ? "," + memberIds.join(",") : "");
+            (some(namedMemberDeclarations, node => !!node) ? "," + map(namedMemberDeclarations, node => node ? getNodeId(node) : "_").join(",") : "");


Is there a reason to precheck rather than just doing the map and letting it give the empty string via returning nothing?

Hmm, not rly - this only keeps the cache key shorter if there are no named members.

Oh, I misread this and didn't notice that this was adding a conditional prefix. Just seemed less nice to double iterate but it's fine.

I debugged this down in a moment between things so I didn't exactly give it an extended thought at that moment. It was previously added conditionally and I didn't spend a second thinking if this would still be needed after things change.

Since you called it out, I re-evaluated it and I think it is, indeed, redundant now. So I would be more than OK with dropping this. Unlabeled tuples are probably more common than labeled ones though but I doubt that the cache key size would impact perf in a significant way.

All good, I'd just leave it as it's not a regression.

jakebailey · 2023-09-11T19:26:51Z

@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

typescript-bot · 2023-09-11T19:26:53Z

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 01cb0ae. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-09-11T19:26:53Z

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 01cb0ae. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-09-11T19:26:53Z

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 01cb0ae. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-09-11T19:26:53Z

Heya @jakebailey, I've started to run the regular perf test suite on this PR at 01cb0ae. You can monitor the build here.

Update: The results are in!

typescript-bot · 2023-09-11T19:46:24Z

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/55695/merge:

There were infrastructure failures potentially unrelated to your change:

1 instance of "Unknown failure"
2 instances of "Package install failed"

Otherwise...

Everything looks good!

typescript-bot · 2023-09-11T19:58:55Z

@jakebailey
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - node (v16.17.1, x64)}
_{Memory used}	_{300,276k (± 0.01%)}	_{300,282k (± 0.01%)}	_~	_300,256k	_300,309k	_{p=0.470 n=6}
_{Parse Time}	_{3.01s (± 0.18%)}	_{3.01s (± 0.30%)}	_~	_3.00s	_3.02s	_{p=0.341 n=6}
_{Bind Time}	_{0.93s (± 0.00%)}	_{0.93s (± 0.00%)}	_~	_0.93s	_0.93s	_{p=1.000 n=6}
_{Check Time}	_{9.31s (± 0.32%)}	_{9.32s (± 0.27%)}	_~	_9.29s	_9.35s	_{p=0.807 n=6}
_{Emit Time}	_{7.61s (± 0.37%)}	_{7.62s (± 0.10%)}	_~	_7.61s	_7.63s	_{p=0.418 n=6}
_{Total Time}	_{20.86s (± 0.24%)}	_{20.88s (± 0.14%)}	_~	_20.83s	_20.90s	_{p=0.627 n=6}
_{Compiler-Unions - node (v16.17.1, x64)}
_{Memory used}	_{193,987k (± 0.02%)}	_{193,926k (± 0.02%)}	_{-61k (- 0.03%)}	_193,852k	_193,972k	_{p=0.031 n=6}
_{Parse Time}	_{1.58s (± 0.52%)}	_{1.58s (± 0.00%)}	_~	_1.58s	_1.58s	_{p=0.405 n=6}
_{Bind Time}	_{0.79s (± 0.69%)}	_{0.79s (± 0.65%)}	_~	_0.79s	_0.80s	_{p=0.640 n=6}
_{Check Time}	_{9.94s (± 0.27%)}	_{9.92s (± 0.50%)}	_~	_9.86s	_9.97s	_{p=0.687 n=6}
_{Emit Time}	_{2.74s (± 0.27%)}	_{2.74s (± 0.30%)}	_~	_2.73s	_2.75s	_{p=0.729 n=6}
_{Total Time}	_{15.05s (± 0.20%)}	_{15.03s (± 0.34%)}	_~	_14.96s	_15.08s	_{p=0.629 n=6}
_{Monaco - node (v16.17.1, x64)}
_{Memory used}	_{347,178k (± 0.01%)}	_{347,172k (± 0.01%)}	_~	_347,108k	_347,196k	_{p=0.378 n=6}
_{Parse Time}	_{2.69s (± 0.15%)}	_{2.68s (± 0.19%)}	_{-0.01s (- 0.31%)}	_2.68s	_2.69s	_{p=0.022 n=6}
_{Bind Time}	_{0.99s (± 0.00%)}	_{0.99s (± 0.00%)}	_~	_0.99s	_0.99s	_{p=1.000 n=6}
_{Check Time}	_{7.94s (± 0.21%)}	_{7.93s (± 0.29%)}	_~	_7.90s	_7.96s	_{p=0.517 n=6}
_{Emit Time}	_{4.27s (± 0.15%)}	_{4.27s (± 0.38%)}	_~	_4.24s	_4.29s	_{p=0.720 n=6}
_{Total Time}	_{15.88s (± 0.12%)}	_{15.87s (± 0.14%)}	_~	_15.84s	_15.90s	_{p=0.145 n=6}
_{TFS - node (v16.17.1, x64)}
_{Memory used}	_{301,182k (± 0.01%)}	_{301,181k (± 0.00%)}	_~	_301,172k	_301,192k	_{p=0.748 n=6}
_{Parse Time}	_{2.18s (± 0.69%)}	_{2.17s (± 0.63%)}	_~	_2.16s	_2.19s	_{p=1.000 n=6}
_{Bind Time}	_{1.11s (± 0.37%)}	_{1.11s (± 0.37%)}	_~	_1.11s	_1.12s	_{p=0.218 n=6}
_{Check Time}	_{7.23s (± 0.24%)}	_{7.22s (± 0.23%)}	_~	_7.20s	_7.24s	_{p=0.415 n=6}
_{Emit Time}	_{3.98s (± 0.48%)}	_{3.98s (± 0.22%)}	_~	_3.97s	_3.99s	_{p=0.934 n=6}
_{Total Time}	_{14.50s (± 0.28%)}	_{14.49s (± 0.16%)}	_~	_14.47s	_14.53s	_{p=0.519 n=6}
_{material-ui - node (v16.17.1, x64)}
_{Memory used}	_{479,482k (± 0.00%)}	_{479,471k (± 0.00%)}	_{-11k (- 0.00%)}	_479,460k	_479,477k	_{p=0.028 n=6}
_{Parse Time}	_{3.15s (± 0.13%)}	_{3.15s (± 0.35%)}	_~	_3.14s	_3.17s	_{p=1.000 n=6}
_{Bind Time}	_{0.91s (± 0.00%)}	_{0.91s (± 0.45%)}	_~	_0.91s	_0.92s	_{p=0.405 n=6}
_{Check Time}	_{17.80s (± 0.29%)}	_{17.76s (± 0.28%)}	_~	_17.67s	_17.82s	_{p=0.141 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{21.86s (± 0.23%)}	_{21.82s (± 0.27%)}	_~	_21.73s	_21.91s	_{p=0.195 n=6}
_{xstate - node (v16.17.1, x64)}
_{Memory used}	_{542,845k (± 0.01%)}	_{542,850k (± 0.01%)}	_~	_542,786k	_542,914k	_{p=0.936 n=6}
_{Parse Time}	_{3.68s (± 0.30%)}	_{3.69s (± 0.38%)}	_~	_3.67s	_3.71s	_{p=0.216 n=6}
_{Bind Time}	_{1.43s (± 3.62%)}	_{1.42s (± 4.13%)}	_~	_1.34s	_1.46s	_{p=0.492 n=6}
_{Check Time}	_{3.18s (± 2.05%)}	_{3.22s (± 2.96%)}	_~	_3.14s	_3.37s	_{p=0.373 n=6}
_{Emit Time}	_{0.09s (± 5.95%)}	_{0.09s (± 6.44%)}	_~	_0.08s	_0.09s	_{p=0.640 n=6}
_{Total Time}	_{8.39s (± 0.36%)}	_{8.42s (± 0.57%)}	_~	_8.36s	_8.50s	_{p=0.296 n=6}

System info unknown

Hosts

node (v16.17.1, x64)

Scenarios

Angular - node (v16.17.1, x64)
Compiler-Unions - node (v16.17.1, x64)
Monaco - node (v16.17.1, x64)
TFS - node (v16.17.1, x64)
material-ui - node (v16.17.1, x64)
xstate - node (v16.17.1, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

tsserver

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Compiler-UnionsTSServer - node (v16.17.1, x64)}
_{Req 1 - updateOpen}	_{2,490ms (± 0.13%)}	_{2,489ms (± 0.20%)}	_~	_2,479ms	_2,492ms	_{p=0.627 n=6}
_{Req 2 - geterr}	_{5,935ms (± 0.32%)}	_{5,933ms (± 0.20%)}	_~	_5,914ms	_5,945ms	_{p=1.000 n=6}
_{Req 3 - references}	_{343ms (± 0.24%)}	_{343ms (± 0.29%)}	_~	_341ms	_344ms	_{p=0.396 n=6}
_{Req 4 - navto}	_{276ms (± 0.72%)}	_{277ms (± 0.44%)}	_~	_276ms	_279ms	_{p=0.108 n=6}
_{Req 5 - completionInfo count}	_{1,356 (± 0.00%)}	_{1,356 (± 0.00%)}	_~	_1,356	_1,356	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{85ms (± 8.41%)}	_{78ms (± 4.31%)}	_~	_76ms	_83ms	_{p=0.113 n=6}
_{CompilerTSServer - node (v16.17.1, x64)}
_{Req 1 - updateOpen}	_{2,622ms (± 0.53%)}	_{2,614ms (± 0.37%)}	_~	_2,607ms	_2,633ms	_{p=0.261 n=6}
_{Req 2 - geterr}	_{4,772ms (± 0.24%)}	_{4,770ms (± 0.28%)}	_~	_4,746ms	_4,787ms	_{p=1.000 n=6}
_{Req 3 - references}	_{351ms (± 0.18%)}	_{351ms (± 0.24%)}	_~	_349ms	_351ms	_{p=0.340 n=6}
_{Req 4 - navto}	_{270ms (± 1.04%)}	_{268ms (± 0.38%)}	_~	_267ms	_269ms	_{p=0.070 n=6}
_{Req 5 - completionInfo count}	_{1,518 (± 0.00%)}	_{1,518 (± 0.00%)}	_~	_1,518	_1,518	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{79ms (± 0.95%)}	_{79ms (± 0.52%)}	_~	_78ms	_79ms	_{p=0.389 n=6}
_{xstateTSServer - node (v16.17.1, x64)}
_{Req 1 - updateOpen}	_{2,705ms (± 0.04%)}	_{2,705ms (± 0.16%)}	_~	_2,698ms	_2,711ms	_{p=0.737 n=6}
_{Req 2 - geterr}	_{1,974ms (± 0.53%)}	_{1,957ms (± 1.06%)}	_~	_1,920ms	_1,980ms	_{p=0.091 n=6}
_{Req 3 - references}	_{140ms (± 1.39%)}	_{139ms (± 2.25%)}	_~	_134ms	_142ms	_{p=1.000 n=6}
_{Req 4 - navto}	_{360ms (± 0.80%)}	_{361ms (± 1.03%)}	_~	_357ms	_365ms	_{p=0.683 n=6}
_{Req 5 - completionInfo count}	_{2,071 (± 0.00%)}	_{2,071 (± 0.00%)}	_~	_2,071	_2,071	_{p=1.000 n=6}
_{Req 5 - completionInfo}	_{324ms (± 1.93%)}	_{324ms (± 1.59%)}	_~	_314ms	_328ms	_{p=0.623 n=6}

System info unknown

Hosts

node (v16.17.1, x64)

Scenarios

CompilerTSServer - node (v16.17.1, x64)
Compiler-UnionsTSServer - node (v16.17.1, x64)
xstateTSServer - node (v16.17.1, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Startup

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{tsc-startup - node (v16.17.1, x64)}
_{Execution time}	_{156.46ms (± 0.22%)}	_{156.47ms (± 0.17%)}	_~	_154.64ms	_158.86ms	_{p=0.579 n=600}
_{tsserver-startup - node (v16.17.1, x64)}
_{Execution time}	_{230.82ms (± 0.12%)}	_{231.52ms (± 0.17%)}	_{+0.70ms (+ 0.30%)}	_229.71ms	_238.86ms	_{p=0.000 n=600}
_{tsserverlibrary-startup - node (v16.17.1, x64)}
_{Execution time}	_{235.75ms (± 0.12%)}	_{236.21ms (± 0.12%)}	_{+0.45ms (+ 0.19%)}	_235.15ms	_242.65ms	_{p=0.000 n=600}
_{typescript-startup - node (v16.17.1, x64)}
_{Execution time}	_{235.89ms (± 0.11%)}	_{235.89ms (± 0.12%)}	_~	_234.81ms	_239.66ms	_{p=0.907 n=600}

System info unknown

Hosts

node (v16.17.1, x64)

Scenarios

tsc-startup - node (v16.17.1, x64)
tsserver-startup - node (v16.17.1, x64)
tsserverlibrary-startup - node (v16.17.1, x64)
typescript-startup - node (v16.17.1, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2023-09-11T20:30:59Z

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/55695/merge:

Everything looks good!

typescript-bot · 2023-09-11T21:38:29Z

Hey @jakebailey, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

jakebailey · 2023-09-11T21:41:40Z

Maybe this might be worth a backport given this can end up breaking the new feature in 5.2? @DanielRosenwasser

…ed members (microsoft#55695)

Fixed cache key computation for tuple target types with partially nam…

01cb0ae

…ed members

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 9, 2023

Andarist commented Sep 9, 2023

View reviewed changes

jakebailey reviewed Sep 9, 2023

View reviewed changes

jakebailey approved these changes Sep 11, 2023

View reviewed changes

jakebailey merged commit 4b0f54f into microsoft:main Sep 11, 2023

snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023

Fixed cache key computation for tuple target types with partially nam…

3ae9594

…ed members (microsoft#55695)

Fixed cache key computation for tuple target types with partially named members #55695

Fixed cache key computation for tuple target types with partially named members #55695

Uh oh!

Conversation

Andarist commented Sep 9, 2023

Uh oh!

Andarist Sep 9, 2023

Choose a reason for hiding this comment

Uh oh!

jakebailey Sep 9, 2023

Choose a reason for hiding this comment

Uh oh!

Andarist Sep 9, 2023

Choose a reason for hiding this comment

Uh oh!

jakebailey Sep 9, 2023

Choose a reason for hiding this comment

Uh oh!

Andarist Sep 10, 2023

Choose a reason for hiding this comment

Uh oh!

jakebailey Sep 10, 2023

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Sep 11, 2023

Uh oh!

typescript-bot commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Sep 11, 2023

Uh oh!

typescript-bot commented Sep 11, 2023

Compiler

tsserver

Startup

Uh oh!

typescript-bot commented Sep 11, 2023

Uh oh!

typescript-bot commented Sep 11, 2023

Uh oh!

jakebailey commented Sep 11, 2023

Uh oh!

Uh oh!

typescript-bot commented Sep 11, 2023 •

edited

Loading

typescript-bot commented Sep 11, 2023 •

edited

Loading

typescript-bot commented Sep 11, 2023 •

edited

Loading

typescript-bot commented Sep 11, 2023 •

edited

Loading