Always cache relations involving intersection types #46523

ahejlsberg · 2021-10-25T22:14:41Z

An alternative fix for #46500, hopefully with less impact on the material-ui performance test.

ahejlsberg · 2021-10-25T22:15:23Z

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster

typescript-bot · 2021-10-25T22:15:26Z

Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 8c278bf. You can monitor the build here.

Update: The results are in!

typescript-bot · 2021-10-25T22:15:26Z

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 8c278bf. You can monitor the build here.

typescript-bot · 2021-10-25T22:15:26Z

Heya @ahejlsberg, I've started to run the inline community code test suite on this PR at 8c278bf. You can monitor the build here.

Update: The results are in!

typescript-bot · 2021-10-25T22:15:27Z

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

typescript-bot · 2021-10-25T22:30:35Z

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

Here they are:

Comparison Report - main..46523

_Metric	_main	₄₆₅₂₃	_Delta	_Best	_Worst
_{Angular - node (v14.15.1, x64)}
_{Memory used}	_{330,354k (± 0.01%)}	_{330,365k (± 0.01%)}	_{+10k (+ 0.00%)}	_330,337k	_330,406k
_{Parse Time}	_{1.95s (± 0.64%)}	_{1.95s (± 0.25%)}	_{+0.00s (+ 0.00%)}	_1.94s	_1.96s
_{Bind Time}	_{0.86s (± 0.69%)}	_{0.86s (± 1.00%)}	_{+0.01s (+ 0.70%)}	_0.85s	_0.89s
_{Check Time}	_{5.33s (± 0.43%)}	_{5.35s (± 0.25%)}	_{+0.02s (+ 0.38%)}	_5.31s	_5.37s
_{Emit Time}	_{6.13s (± 0.50%)}	_{6.12s (± 0.56%)}	_{-0.01s (- 0.18%)}	_6.04s	_6.19s
_{Total Time}	_{14.26s (± 0.26%)}	_{14.27s (± 0.31%)}	_{+0.02s (+ 0.11%)}	_14.19s	_14.34s
_{Compiler-Unions - node (v14.15.1, x64)}
_{Memory used}	_{192,567k (± 0.49%)}	_{191,806k (± 0.69%)}	_{-761k (- 0.40%)}	_189,076k	_193,274k
_{Parse Time}	_{0.81s (± 0.69%)}	_{0.81s (± 0.82%)}	_{+0.00s (+ 0.25%)}	_0.80s	_0.83s
_{Bind Time}	_{0.55s (± 0.40%)}	_{0.55s (± 0.66%)}	_{+0.00s (+ 0.54%)}	_0.55s	_0.56s
_{Check Time}	_{7.53s (± 0.44%)}	_{7.54s (± 0.41%)}	_{+0.01s (+ 0.13%)}	_7.48s	_7.63s
_{Emit Time}	_{2.44s (± 0.71%)}	_{2.44s (± 1.00%)}	_{-0.01s (- 0.29%)}	_2.38s	_2.48s
_{Total Time}	_{11.34s (± 0.42%)}	_{11.34s (± 0.45%)}	_{+0.01s (+ 0.04%)}	_11.20s	_11.46s
_{Monaco - node (v14.15.1, x64)}
_{Memory used}	_{324,010k (± 0.01%)}	_{323,995k (± 0.01%)}	_{-15k (- 0.00%)}	_323,947k	_324,032k
_{Parse Time}	_{1.51s (± 0.62%)}	_{1.50s (± 0.33%)}	_{-0.00s (- 0.20%)}	_1.49s	_1.51s
_{Bind Time}	_{0.75s (± 0.48%)}	_{0.75s (± 0.53%)}	_{-0.01s (- 0.66%)}	_0.74s	_0.76s
_{Check Time}	_{5.30s (± 0.42%)}	_{5.29s (± 0.18%)}	_{-0.01s (- 0.11%)}	_5.27s	_5.32s
_{Emit Time}	_{3.20s (± 0.53%)}	_{3.21s (± 0.34%)}	_{+0.01s (+ 0.41%)}	_3.19s	_3.24s
_{Total Time}	_{10.76s (± 0.31%)}	_{10.76s (± 0.13%)}	_{-0.00s (- 0.04%)}	_10.73s	_10.80s
_{TFS - node (v14.15.1, x64)}
_{Memory used}	_{288,365k (± 0.01%)}	_{288,366k (± 0.01%)}	_{+1k (+ 0.00%)}	_288,324k	_288,404k
_{Parse Time}	_{1.23s (± 0.70%)}	_{1.22s (± 0.67%)}	_{-0.01s (- 0.73%)}	_1.21s	_1.25s
_{Bind Time}	_{0.73s (± 0.76%)}	_{0.73s (± 0.45%)}	_{-0.00s (- 0.27%)}	_0.72s	_0.74s
_{Check Time}	_{4.90s (± 0.53%)}	_{4.90s (± 0.50%)}	_{+0.01s (+ 0.12%)}	_4.85s	_4.95s
_{Emit Time}	_{3.48s (± 0.69%)}	_{3.48s (± 0.68%)}	_{+0.00s (+ 0.12%)}	_3.44s	_3.54s
_{Total Time}	_{10.34s (± 0.35%)}	_{10.34s (± 0.46%)}	_{+0.00s (+ 0.01%)}	_10.25s	_10.43s
_{material-ui - node (v14.15.1, x64)}
_{Memory used}	_{447,290k (± 0.08%)}	_{448,412k (± 0.06%)}	_{+1,122k (+ 0.25%)}	_447,375k	_448,582k
_{Parse Time}	_{1.82s (± 0.49%)}	_{1.83s (± 0.69%)}	_{+0.00s (+ 0.05%)}	_1.80s	_1.86s
_{Bind Time}	_{0.68s (± 0.54%)}	_{0.68s (± 0.54%)}	_{0.00s ( 0.00%)}	_0.67s	_0.68s
_{Check Time}	_{13.09s (± 0.97%)}	_{13.07s (± 0.48%)}	_{-0.03s (- 0.19%)}	_12.93s	_13.26s
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_{0.00s ( NaN%)}	_0.00s	_0.00s
_{Total Time}	_{15.59s (± 0.84%)}	_{15.57s (± 0.39%)}	_{-0.02s (- 0.15%)}	_15.42s	_15.76s
_{xstate - node (v14.15.1, x64)}
_{Memory used}	_{533,836k (± 0.01%)}	_{534,053k (± 0.01%)}	_{+217k (+ 0.04%)}	_534,016k	_534,150k
_{Parse Time}	_{2.55s (± 0.54%)}	_{2.56s (± 0.52%)}	_{+0.01s (+ 0.39%)}	_2.54s	_2.59s
_{Bind Time}	_{1.15s (± 0.82%)}	_{1.16s (± 1.25%)}	_{+0.01s (+ 0.61%)}	_1.13s	_1.20s
_{Check Time}	_{1.55s (± 0.54%)}	_{1.54s (± 0.32%)}	_{-0.00s (- 0.00%)}	_1.54s	_1.56s
_{Emit Time}	_{0.07s (± 0.00%)}	_{0.07s (± 0.00%)}	_{0.00s ( 0.00%)}	_0.07s	_0.07s
_{Total Time}	_{5.32s (± 0.36%)}	_{5.33s (± 0.43%)}	_{+0.01s (+ 0.21%)}	_5.28s	_5.40s

System

_{Machine Name}	_ts-ci-ubuntu
_Platform	_{linux 4.4.0-210-generic}
_Architecture	_x64
_{Available Memory}	_{16 GB}
_{Available Memory}	_{10 GB}
_CPUs	_{4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz}

Hosts

node (v14.15.1, x64)

Scenarios

Angular - node (v14.15.1, x64)
Compiler-Unions - node (v14.15.1, x64)
Monaco - node (v14.15.1, x64)
TFS - node (v14.15.1, x64)
material-ui - node (v14.15.1, x64)
xstate - node (v14.15.1, x64)

_Benchmark	_Name	_Iterations
_Current	₄₆₅₂₃	₁₀
_Baseline	_main	₁₀

Developer Information:

Download Benchmark

typescript-bot · 2021-10-25T22:33:32Z

@ahejlsberg
Great news! no new errors were found between main..refs/pull/46523/merge

ahejlsberg · 2021-10-25T23:01:10Z

No effect on performance from this one, so definitely a better fix than #46505.

andrewbranch · 2021-10-25T23:15:43Z

tests/baselines/reference/unionTypeCallSignatures6.errors.txt

@@ -94,5 +91,4 @@ tests/cases/conformance/types/union/unionTypeCallSignatures6.ts(55,1): error TS2
 !!! error TS2769:       Type 'void' is not assignable to type 'A'.
 !!! error TS2769:   Overload 2 of 2, '(this: A & B): void', gave the following error.
 !!! error TS2769:     The 'this' context of type 'void' is not assignable to method's 'this' of type 'A & B'.
-!!! error TS2769:       Type 'void' is not assignable to type 'A'.


The loss of this ending of the elaboration is a little unfortunate since the line above it is kind of an earfull.

Yeah, but that's basically the effect we get from caching the relation.

weswigham

Huh, this actually reduced memory usage for Compiler - Unions. We sure we can't also just cache all union relations as well, and not just those involving more than 4 union members? It'd simplify the code a bit.

ahejlsberg · 2021-10-26T00:52:19Z

@weswigham We already tried that back when, but let's get some fresh data: #46528.

ahejlsberg · 2021-10-26T16:35:54Z

Well, #46528 doesn't indicate much of a difference, but I'm going to be conservative and keep the non-caching behavior for small unions.

DanielRosenwasser · 2021-11-08T08:12:32Z

Was this meant to be picked into 4.5, or was it intentionally 4.6? The issue's milestone is 4.5.1.

ahejlsberg · 2021-11-08T18:55:05Z

I think we should pick it for 4.5 if possible. It's a 4.5 specific regression, probably not too common, but hard to know. I'd say the fix is very safe indeed.

DanielRosenwasser · 2021-11-08T21:55:12Z

@typescript-bot cherry-pick this to release-4.5

typescript-bot · 2021-11-08T21:55:14Z

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-4.5 on this PR at 8c278bf. You can monitor the build here.

typescript-bot · 2021-11-08T21:59:20Z

Hey @DanielRosenwasser, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-4.5 manually.

* Always cache relations involving intersection types * Accept new baselines * Add regression test

ahejlsberg added 3 commits October 25, 2021 15:05

Always cache relations involving intersection types

79a73bf

Accept new baselines

bd0833a

Add regression test

8c278bf

typescript-bot assigned ahejlsberg Oct 25, 2021

typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 25, 2021

ahejlsberg mentioned this pull request Oct 25, 2021

Still check for deep nesting when bypassing relation caching #46505

Closed

ahejlsberg requested review from andrewbranch and weswigham October 25, 2021 22:40

andrewbranch approved these changes Oct 25, 2021

View reviewed changes

weswigham approved these changes Oct 25, 2021

View reviewed changes

ahejlsberg merged commit 44c63a7 into main Oct 26, 2021

ahejlsberg deleted the fix46500-2 branch October 26, 2021 16:36

mprobst pushed a commit to mprobst/TypeScript that referenced this pull request Jan 10, 2022

Always cache relations involving intersection types (microsoft#46523)

9c4d9da

* Always cache relations involving intersection types * Accept new baselines * Add regression test

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always cache relations involving intersection types #46523

Always cache relations involving intersection types #46523

ahejlsberg commented Oct 25, 2021

ahejlsberg commented Oct 25, 2021

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021

typescript-bot commented Oct 25, 2021

ahejlsberg commented Oct 25, 2021

andrewbranch Oct 25, 2021

ahejlsberg Oct 26, 2021

weswigham left a comment

ahejlsberg commented Oct 26, 2021

ahejlsberg commented Oct 26, 2021

DanielRosenwasser commented Nov 8, 2021

ahejlsberg commented Nov 8, 2021

DanielRosenwasser commented Nov 8, 2021

typescript-bot commented Nov 8, 2021 •

edited

Loading

typescript-bot commented Nov 8, 2021

Always cache relations involving intersection types #46523

Always cache relations involving intersection types #46523

Conversation

ahejlsberg commented Oct 25, 2021

ahejlsberg commented Oct 25, 2021

typescript-bot commented Oct 25, 2021 • edited Loading

typescript-bot commented Oct 25, 2021 • edited Loading

typescript-bot commented Oct 25, 2021 • edited Loading

typescript-bot commented Oct 25, 2021 • edited Loading

typescript-bot commented Oct 25, 2021

typescript-bot commented Oct 25, 2021

ahejlsberg commented Oct 25, 2021

andrewbranch Oct 25, 2021

Choose a reason for hiding this comment

ahejlsberg Oct 26, 2021

Choose a reason for hiding this comment

weswigham left a comment

Choose a reason for hiding this comment

ahejlsberg commented Oct 26, 2021

ahejlsberg commented Oct 26, 2021

DanielRosenwasser commented Nov 8, 2021

ahejlsberg commented Nov 8, 2021

DanielRosenwasser commented Nov 8, 2021

typescript-bot commented Nov 8, 2021 • edited Loading

typescript-bot commented Nov 8, 2021

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Oct 25, 2021 •

edited

Loading

typescript-bot commented Nov 8, 2021 •

edited

Loading