Skip to content

Fix recursive type inference #53396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 21, 2023
Merged

Fix recursive type inference #53396

merged 6 commits into from
Mar 21, 2023

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Mar 20, 2023

This PR reverts #49226 and instead fixes the problem as described here.

Fixes #48524.
Fixes #52722.

@ahejlsberg
Copy link
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 20, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 20, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 20, 2023

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 20, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 20, 2023

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

Update: The results are in!

@@ -249,7 +249,7 @@ declare let z: Box2<Box2<string>>;
>z : Box2<Box2<string>>

foo(z); // unknown, but ideally would be string (requires unique recursion ID for each type reference)
Copy link
Member

Choose a reason for hiding this comment

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

Comment can probably be removed here now that this is saying string?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Neat that we already had a test to show the issue.

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@MariaSolOs
Copy link
Contributor

Oh, could this also fix what I was working on in #53327?

@typescript-bot
Copy link
Collaborator

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

@ahejlsberg
Copy link
Member Author

@MariaSolOs I doubt it, unfortunately.

@ahejlsberg
Copy link
Member Author

Waiting on the performance suite, everything else looks good.

@ahejlsberg
Copy link
Member Author

@typescript-bot perf test faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 21, 2023

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

Update: The results are in!

@jakebailey
Copy link
Member

The original run is still queued; we just have a huge perf backlog and that's why it hasn't shown up yet.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..53396

Metric main 53396 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 361,038k (± 0.01%) 361,427k (± 0.01%) +389k (+ 0.11%) 361,368k 361,468k p=0.005 n=6
Parse Time 3.52s (± 0.21%) 3.55s (± 0.74%) ~ 3.51s 3.58s p=0.050 n=6
Bind Time 1.18s (± 0.44%) 1.18s (± 0.44%) ~ 1.18s 1.19s p=1.000 n=6
Check Time 9.50s (± 0.57%) 9.54s (± 0.55%) ~ 9.49s 9.62s p=0.258 n=6
Emit Time 7.94s (± 0.62%) 8.00s (± 0.85%) ~ 7.89s 8.09s p=0.227 n=6
Total Time 22.14s (± 0.30%) 22.27s (± 0.27%) +0.13s (+ 0.56%) 22.19s 22.34s p=0.020 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,705k (± 0.89%) 193,081k (± 0.68%) ~ 192,478k 195,757k p=0.230 n=6
Parse Time 1.58s (± 1.01%) 1.60s (± 0.39%) +0.02s (+ 1.16%) 1.59s 1.61s p=0.009 n=6
Bind Time 0.82s (± 1.27%) 0.83s (± 0.66%) ~ 0.82s 0.83s p=1.000 n=6
Check Time 10.11s (± 0.57%) 10.16s (± 0.59%) ~ 10.11s 10.28s p=0.090 n=6
Emit Time 2.98s (± 0.55%) 2.99s (± 0.74%) ~ 2.96s 3.01s p=0.255 n=6
Total Time 15.49s (± 0.35%) 15.58s (± 0.47%) ~ 15.53s 15.72s p=0.050 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,561k (± 0.00%) 345,564k (± 0.00%) ~ 345,539k 345,580k p=0.630 n=6
Parse Time 2.73s (± 0.55%) 2.73s (± 0.68%) ~ 2.71s 2.76s p=0.459 n=6
Bind Time 1.09s (± 0.96%) 1.09s (± 0.00%) ~ 1.09s 1.09s p=0.293 n=6
Check Time 7.73s (± 0.54%) 7.70s (± 0.49%) ~ 7.66s 7.76s p=0.334 n=6
Emit Time 4.47s (± 0.28%) 4.46s (± 0.59%) ~ 4.42s 4.49s p=0.743 n=6
Total Time 16.02s (± 0.36%) 15.99s (± 0.16%) ~ 15.96s 16.03s p=0.686 n=6
TFS - node (v16.17.1, x64)
Memory used 299,817k (± 0.01%) 299,808k (± 0.01%) ~ 299,752k 299,861k p=0.520 n=6
Parse Time 2.18s (± 0.95%) 2.17s (± 0.54%) ~ 2.15s 2.18s p=0.286 n=6
Bind Time 1.24s (± 1.10%) 1.24s (± 0.88%) ~ 1.22s 1.25s p=0.611 n=6
Check Time 7.18s (± 0.38%) 7.21s (± 0.47%) ~ 7.15s 7.25s p=0.124 n=6
Emit Time 4.34s (± 0.86%) 4.33s (± 0.86%) ~ 4.29s 4.38s p=0.520 n=6
Total Time 14.95s (± 0.51%) 14.94s (± 0.35%) ~ 14.89s 15.04s p=1.000 n=6
material-ui - node (v16.17.1, x64)
Memory used 476,401k (± 0.00%) 476,437k (± 0.00%) +36k (+ 0.01%) 476,417k 476,451k p=0.020 n=6
Parse Time 3.21s (± 0.42%) 3.22s (± 0.23%) ~ 3.21s 3.23s p=0.111 n=6
Bind Time 0.96s (± 0.54%) 0.96s (± 0.54%) ~ 0.95s 0.96s p=1.000 n=6
Check Time 18.01s (± 0.70%) 18.15s (± 0.38%) +0.14s (+ 0.80%) 18.07s 18.28s p=0.045 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.18s (± 0.65%) 22.34s (± 0.31%) +0.16s (+ 0.70%) 22.25s 22.46s p=0.045 n=6
xstate - node (v16.17.1, x64)
Memory used 550,557k (± 0.02%) 550,546k (± 0.01%) ~ 550,507k 550,671k p=0.810 n=6
Parse Time 3.95s (± 0.46%) 3.95s (± 0.36%) ~ 3.93s 3.97s p=0.743 n=6
Bind Time 1.80s (± 0.45%) 1.80s (± 0.35%) ~ 1.79s 1.81s p=0.432 n=6
Check Time 2.99s (± 0.82%) 3.03s (± 0.87%) ~ 3.00s 3.07s p=0.053 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.83s (± 0.35%) 8.87s (± 0.39%) ~ 8.84s 8.92s p=0.089 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
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 53396 6
Baseline main 6

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..53396

Metric main 53396 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 361,392k (± 0.01%) 361,054k (± 0.01%) -338k (- 0.09%) 361,003k 361,093k p=0.005 n=6
Parse Time 3.54s (± 0.60%) 3.53s (± 0.95%) ~ 3.48s 3.58s p=0.624 n=6
Bind Time 1.18s (± 0.71%) 1.18s (± 0.83%) ~ 1.17s 1.19s p=0.734 n=6
Check Time 9.49s (± 0.44%) 9.46s (± 0.24%) ~ 9.42s 9.49s p=0.462 n=6
Emit Time 7.92s (± 0.37%) 7.92s (± 0.59%) ~ 7.85s 7.99s p=0.872 n=6
Total Time 22.13s (± 0.22%) 22.10s (± 0.23%) ~ 22.00s 22.15s p=0.333 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 192,532k (± 0.03%) 192,621k (± 0.02%) +90k (+ 0.05%) 192,537k 192,677k p=0.045 n=6
Parse Time 1.60s (± 0.85%) 1.59s (± 1.82%) ~ 1.54s 1.61s p=0.676 n=6
Bind Time 0.83s (± 0.62%) 0.82s (± 0.50%) -0.01s (- 1.01%) 0.81s 0.82s p=0.022 n=6
Check Time 10.18s (± 0.45%) 10.13s (± 0.41%) ~ 10.09s 10.18s p=0.145 n=6
Emit Time 2.99s (± 1.21%) 2.98s (± 0.72%) ~ 2.96s 3.02s p=0.629 n=6
Total Time 15.60s (± 0.30%) 15.51s (± 0.44%) ~ 15.45s 15.62s p=0.053 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,562k (± 0.01%) 345,536k (± 0.01%) ~ 345,495k 345,557k p=0.066 n=6
Parse Time 2.73s (± 0.50%) 2.73s (± 0.60%) ~ 2.71s 2.76s p=0.325 n=6
Bind Time 1.09s (± 0.47%) 1.09s (± 0.90%) ~ 1.08s 1.11s p=0.386 n=6
Check Time 7.76s (± 0.52%) 7.70s (± 0.20%) -0.06s (- 0.73%) 7.68s 7.72s p=0.024 n=6
Emit Time 4.48s (± 0.58%) 4.45s (± 0.65%) ~ 4.41s 4.48s p=0.076 n=6
Total Time 16.06s (± 0.35%) 15.97s (± 0.21%) -0.08s (- 0.52%) 15.93s 16.01s p=0.013 n=6
TFS - node (v16.17.1, x64)
Memory used 299,778k (± 0.01%) 299,810k (± 0.01%) +33k (+ 0.01%) 299,779k 299,839k p=0.045 n=6
Parse Time 2.17s (± 0.86%) 2.17s (± 0.41%) ~ 2.16s 2.18s p=0.683 n=6
Bind Time 1.23s (± 1.03%) 1.24s (± 0.94%) ~ 1.22s 1.25s p=0.155 n=6
Check Time 7.20s (± 0.39%) 7.21s (± 1.49%) ~ 7.14s 7.43s p=0.518 n=6
Emit Time 4.34s (± 0.50%) 4.38s (± 3.32%) ~ 4.29s 4.67s p=0.466 n=6
Total Time 14.93s (± 0.32%) 15.01s (± 1.77%) ~ 14.84s 15.54s p=0.687 n=6
material-ui - node (v16.17.1, x64)
Memory used 476,489k (± 0.02%) 476,427k (± 0.00%) ~ 476,402k 476,464k p=0.575 n=6
Parse Time 3.22s (± 1.21%) 3.21s (± 0.33%) ~ 3.20s 3.23s p=0.324 n=6
Bind Time 0.98s (± 3.78%) 0.95s (± 0.43%) 🟩-0.03s (- 3.06%) 0.95s 0.96s p=0.007 n=6
Check Time 18.00s (± 1.06%) 18.13s (± 0.32%) ~ 18.04s 18.20s p=0.378 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.20s (± 1.03%) 22.30s (± 0.26%) ~ 22.22s 22.36s p=0.575 n=6
xstate - node (v16.17.1, x64)
Memory used 550,558k (± 0.01%) 550,611k (± 0.02%) ~ 550,474k 550,752k p=0.378 n=6
Parse Time 3.95s (± 0.34%) 3.95s (± 0.21%) ~ 3.94s 3.96s p=0.805 n=6
Bind Time 1.79s (± 0.47%) 1.80s (± 0.30%) ~ 1.79s 1.80s p=0.855 n=6
Check Time 2.99s (± 0.47%) 3.01s (± 0.53%) +0.02s (+ 0.72%) 3.00s 3.04s p=0.041 n=6
Emit Time 0.09s (± 0.00%) 0.09s (± 0.00%) ~ 0.09s 0.09s p=1.000 n=6
Total Time 8.83s (± 0.34%) 8.85s (± 0.20%) ~ 8.83s 8.88s p=0.195 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
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 53396 6
Baseline main 6

Developer Information:

Download Benchmark

@ahejlsberg ahejlsberg merged commit 3d2c344 into main Mar 21, 2023
@ahejlsberg ahejlsberg deleted the fixRecursiveTypeInference branch March 21, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Backlog Bug PRs that fix a backlog bug
Projects
None yet
5 participants