Skip to content

Fix blocking of recursive dependencies in getNarrowedTypeOfSymbol #48941

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 2 commits into from
May 3, 2022

Conversation

ahejlsberg
Copy link
Member

Fixes #48902.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels May 3, 2022
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Thanks!

@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
Copy link
Collaborator

typescript-bot commented May 3, 2022

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 3, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 3, 2022

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 3, 2022

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..48941

Metric main 48941 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 333,432k (± 0.01%) 333,435k (± 0.01%) +3k (+ 0.00%) 333,381k 333,471k
Parse Time 2.02s (± 0.70%) 2.03s (± 0.53%) +0.01s (+ 0.30%) 2.01s 2.05s
Bind Time 0.87s (± 0.77%) 0.88s (± 0.56%) +0.00s (+ 0.34%) 0.86s 0.88s
Check Time 5.63s (± 0.37%) 5.65s (± 0.36%) +0.02s (+ 0.28%) 5.60s 5.70s
Emit Time 6.31s (± 0.50%) 6.36s (± 0.62%) +0.05s (+ 0.79%) 6.27s 6.45s
Total Time 14.84s (± 0.35%) 14.92s (± 0.44%) +0.08s (+ 0.51%) 14.79s 15.05s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,034k (± 0.13%) 192,842k (± 0.51%) +808k (+ 0.42%) 192,055k 195,530k
Parse Time 0.86s (± 0.43%) 0.85s (± 0.76%) -0.00s (- 0.47%) 0.84s 0.86s
Bind Time 0.56s (± 0.59%) 0.56s (± 0.93%) +0.00s (+ 0.18%) 0.55s 0.58s
Check Time 7.48s (± 0.59%) 7.49s (± 0.46%) +0.01s (+ 0.15%) 7.41s 7.56s
Emit Time 2.53s (± 1.21%) 2.51s (± 0.88%) -0.01s (- 0.55%) 2.47s 2.57s
Total Time 11.43s (± 0.46%) 11.42s (± 0.43%) -0.01s (- 0.08%) 11.33s 11.52s
Monaco - node (v14.15.1, x64)
Memory used 325,505k (± 0.01%) 325,478k (± 0.00%) -27k (- 0.01%) 325,454k 325,534k
Parse Time 1.57s (± 0.63%) 1.58s (± 0.92%) +0.00s (+ 0.25%) 1.56s 1.62s
Bind Time 0.77s (± 0.47%) 0.78s (± 0.63%) +0.00s (+ 0.26%) 0.77s 0.79s
Check Time 5.56s (± 0.53%) 5.54s (± 0.41%) -0.02s (- 0.32%) 5.50s 5.60s
Emit Time 3.32s (± 0.55%) 3.33s (± 0.68%) +0.01s (+ 0.33%) 3.29s 3.40s
Total Time 11.23s (± 0.40%) 11.23s (± 0.48%) +0.00s (+ 0.01%) 11.13s 11.40s
TFS - node (v14.15.1, x64)
Memory used 289,078k (± 0.00%) 289,064k (± 0.01%) -15k (- 0.01%) 288,995k 289,106k
Parse Time 1.37s (± 1.11%) 1.36s (± 1.03%) -0.01s (- 0.44%) 1.34s 1.41s
Bind Time 0.72s (± 0.41%) 0.73s (± 1.04%) +0.00s (+ 0.55%) 0.71s 0.75s
Check Time 5.20s (± 0.44%) 5.20s (± 0.44%) +0.00s (+ 0.06%) 5.14s 5.24s
Emit Time 3.54s (± 1.76%) 3.54s (± 2.25%) +0.01s (+ 0.20%) 3.39s 3.70s
Total Time 10.83s (± 0.75%) 10.83s (± 0.88%) +0.00s (+ 0.02%) 10.64s 11.00s
material-ui - node (v14.15.1, x64)
Memory used 447,804k (± 0.00%) 447,829k (± 0.01%) +25k (+ 0.01%) 447,757k 447,897k
Parse Time 1.86s (± 0.62%) 1.87s (± 0.46%) +0.00s (+ 0.16%) 1.84s 1.88s
Bind Time 0.70s (± 0.95%) 0.70s (± 0.68%) -0.00s (- 0.43%) 0.69s 0.71s
Check Time 13.04s (± 0.57%) 13.05s (± 0.81%) +0.01s (+ 0.09%) 12.90s 13.29s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.60s (± 0.56%) 15.61s (± 0.70%) +0.01s (+ 0.08%) 15.46s 15.86s
xstate - node (v14.15.1, x64)
Memory used 535,910k (± 0.00%) 535,005k (± 0.00%) -905k (- 0.17%) 534,953k 535,060k
Parse Time 2.59s (± 0.52%) 2.59s (± 0.56%) -0.00s (- 0.15%) 2.55s 2.61s
Bind Time 1.14s (± 0.65%) 1.14s (± 0.39%) -0.01s (- 0.44%) 1.13s 1.15s
Check Time 1.51s (± 0.38%) 1.51s (± 0.44%) -0.00s (- 0.20%) 1.50s 1.52s
Emit Time 0.07s (± 0.00%) 0.07s (± 6.19%) +0.00s (+ 2.86%) 0.07s 0.09s
Total Time 5.32s (± 0.22%) 5.32s (± 0.31%) 0.00s ( 0.00%) 5.27s 5.35s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory2 GB
CPUs4 × 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 48941 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

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

@DanielRosenwasser
Copy link
Member

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 3, 2022

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

@typescript-bot
Copy link
Collaborator

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.6 manually.

@DanielRosenwasser
Copy link
Member

Really? You couldn't cherry-pick a one-line change? 🙄

@DanielRosenwasser
Copy link
Member

I'll cherry-pick the squash commit manually.

@ahejlsberg
Copy link
Member Author

Tests and perf all look fine.

@ahejlsberg ahejlsberg merged commit 38c1460 into main May 3, 2022
@ahejlsberg ahejlsberg deleted the fix48902 branch May 3, 2022 21:20
DanielRosenwasser pushed a commit that referenced this pull request May 3, 2022
…8941)

* Better blocking of recursive dependencies in getNarrowedTypeOfSymbol

* Add regression test
DanielRosenwasser added a commit that referenced this pull request May 3, 2022
…8941) (#48943)

* Better blocking of recursive dependencies in getNarrowedTypeOfSymbol

* Add regression test

Co-authored-by: Anders Hejlsberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perf regression in object destructuring with default values referencing other destructured members
4 participants