Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revert assignability cases in getNarrowedType #42231

Merged
merged 4 commits into from Feb 9, 2021
Merged

Revert assignability cases in getNarrowedType #42231

merged 4 commits into from Feb 9, 2021

Conversation

orta
Copy link
Contributor

@orta orta commented Jan 6, 2021

Added in #39258 - which caused too many breaks to ship, then skipped in #41849 as the assignability narrowing might still be valuable. However, we've got enough reports that this is probably worth dropping too.

re: #42225 - #41808 #41984

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 6, 2021
@orta orta requested a review from weswigham January 6, 2021 18:18
@orta orta added this to the TypeScript 4.2.0 milestone Jan 6, 2021
@weswigham
Copy link
Member

weswigham commented Jan 6, 2021 •
edited

All of those reports seem to involve Array.isArray which was already reverted - Do we have a new test that can demonstrate what the desired behavior that motivates this change is? Since there's no new test with this PR?

@orta
Copy link
Contributor Author

orta commented Jan 12, 2021

I think this still triggers odd outcomes:

@orta orta mentioned this pull request Jan 13, 2021
Closed
@sandersn sandersn added this to Not started in PR Backlog Jan 20, 2021
@sandersn sandersn moved this from Not started to Needs review in PR Backlog Jan 20, 2021
@sandersn sandersn moved this from Needs review to Waiting on author in PR Backlog Jan 20, 2021
@orta
Copy link
Contributor Author

orta commented Feb 9, 2021

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

Heya @orta, I've started to run the tarball bundle task on this PR at 21d55ad. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

Heya @orta, I've started to run the perf test suite on this PR at 21d55ad. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

Heya @orta, I've started to run the parallelized community code test suite on this PR at 21d55ad. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2021 •
edited

Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/95375/artifacts?artifactName=tgz&fileId=C5F3AF448AAB717F04287FA15ADBF0B42820FB5E42B06DE060336F4780B61B7602&fileName=/typescript-4.2.0-insiders.20210209.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.2.0-pr-42231-8".;

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..42231

Metric master 42231 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 345,671k (± 0.03%) 345,668k (± 0.02%) -3k (- 0.00%) 345,523k 345,776k
Parse Time 1.91s (± 0.65%) 1.91s (± 0.52%) -0.00s (- 0.10%) 1.89s 1.93s
Bind Time 0.82s (± 0.72%) 0.82s (± 0.99%) -0.00s (- 0.12%) 0.81s 0.84s
Check Time 4.96s (± 0.67%) 4.95s (± 0.50%) -0.01s (- 0.30%) 4.89s 5.01s
Emit Time 5.26s (± 0.48%) 5.27s (± 0.46%) +0.01s (+ 0.13%) 5.21s 5.33s
Total Time 12.96s (± 0.35%) 12.95s (± 0.36%) -0.01s (- 0.11%) 12.80s 13.02s
Compiler-Unions - node (v10.16.3, x64)
Memory used 214,423k (± 0.06%) 214,434k (± 0.07%) +10k (+ 0.00%) 213,914k 214,697k
Parse Time 0.78s (± 0.64%) 0.78s (± 0.63%) +0.00s (+ 0.13%) 0.77s 0.79s
Bind Time 0.50s (± 0.73%) 0.50s (± 1.54%) -0.00s (- 0.20%) 0.48s 0.51s
Check Time 10.60s (± 0.72%) 10.63s (± 0.74%) +0.02s (+ 0.23%) 10.46s 10.86s
Emit Time 2.35s (± 0.99%) 2.33s (± 1.32%) -0.01s (- 0.55%) 2.28s 2.41s
Total Time 14.22s (± 0.48%) 14.23s (± 0.58%) +0.01s (+ 0.09%) 14.05s 14.44s
Monaco - node (v10.16.3, x64)
Memory used 355,231k (± 0.03%) 355,290k (± 0.02%) +59k (+ 0.02%) 355,121k 355,417k
Parse Time 1.55s (± 0.57%) 1.55s (± 0.64%) +0.00s (+ 0.00%) 1.53s 1.57s
Bind Time 0.72s (± 0.77%) 0.72s (± 0.90%) +0.00s (+ 0.00%) 0.71s 0.74s
Check Time 5.10s (± 0.41%) 5.10s (± 0.56%) -0.00s (- 0.02%) 5.07s 5.20s
Emit Time 2.81s (± 0.68%) 2.79s (± 0.81%) -0.02s (- 0.75%) 2.74s 2.85s
Total Time 10.19s (± 0.37%) 10.17s (± 0.49%) -0.02s (- 0.24%) 10.08s 10.32s
TFS - node (v10.16.3, x64)
Memory used 308,145k (± 0.02%) 308,198k (± 0.02%) +53k (+ 0.02%) 308,110k 308,325k
Parse Time 1.20s (± 0.50%) 1.22s (± 0.92%) +0.01s (+ 0.83%) 1.19s 1.25s
Bind Time 0.68s (± 0.69%) 0.68s (± 0.73%) +0.00s (+ 0.74%) 0.68s 0.70s
Check Time 4.59s (± 0.40%) 4.62s (± 0.58%) +0.03s (+ 0.59%) 4.55s 4.70s
Emit Time 2.96s (± 1.20%) 2.95s (± 1.04%) -0.02s (- 0.57%) 2.87s 3.02s
Total Time 9.43s (± 0.46%) 9.46s (± 0.54%) +0.03s (+ 0.29%) 9.38s 9.62s
material-ui - node (v10.16.3, x64)
Memory used 495,568k (± 0.02%) 495,574k (± 0.01%) +6k (+ 0.00%) 495,477k 495,702k
Parse Time 1.99s (± 0.82%) 1.97s (± 0.67%) -0.02s (- 0.95%) 1.95s 2.01s
Bind Time 0.66s (± 0.91%) 0.65s (± 0.89%) -0.00s (- 0.31%) 0.64s 0.67s
Check Time 13.96s (± 0.55%) 13.88s (± 0.33%) -0.08s (- 0.56%) 13.76s 13.95s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.61s (± 0.52%) 16.51s (± 0.31%) -0.10s (- 0.60%) 16.37s 16.61s
Angular - node (v12.1.0, x64)
Memory used 323,216k (± 0.03%) 323,245k (± 0.03%) +29k (+ 0.01%) 323,112k 323,563k
Parse Time 1.91s (± 0.61%) 1.91s (± 0.73%) +0.00s (+ 0.00%) 1.88s 1.94s
Bind Time 0.80s (± 0.72%) 0.80s (± 1.06%) -0.00s (- 0.50%) 0.78s 0.82s
Check Time 4.86s (± 0.62%) 4.88s (± 1.25%) +0.02s (+ 0.47%) 4.81s 5.09s
Emit Time 5.46s (± 1.50%) 5.44s (± 1.15%) -0.02s (- 0.44%) 5.33s 5.63s
Total Time 13.03s (± 0.70%) 13.03s (± 0.70%) -0.00s (- 0.02%) 12.85s 13.22s
Compiler-Unions - node (v12.1.0, x64)
Memory used 199,843k (± 0.07%) 199,848k (± 0.06%) +6k (+ 0.00%) 199,519k 200,033k
Parse Time 0.77s (± 0.97%) 0.76s (± 0.95%) -0.01s (- 1.42%) 0.75s 0.79s
Bind Time 0.50s (± 1.04%) 0.50s (± 0.80%) 0.00s ( 0.00%) 0.49s 0.51s
Check Time 9.76s (± 1.17%) 9.67s (± 0.89%) -0.09s (- 0.92%) 9.46s 9.92s
Emit Time 2.37s (± 1.12%) 2.35s (± 1.02%) -0.02s (- 0.84%) 2.28s 2.40s
Total Time 13.40s (± 1.00%) 13.28s (± 0.71%) -0.12s (- 0.90%) 13.07s 13.56s
Monaco - node (v12.1.0, x64)
Memory used 337,509k (± 0.02%) 337,503k (± 0.01%) -5k (- 0.00%) 337,432k 337,614k
Parse Time 1.53s (± 0.92%) 1.52s (± 0.44%) -0.01s (- 0.65%) 1.51s 1.54s
Bind Time 0.71s (± 0.52%) 0.71s (± 0.70%) -0.00s (- 0.00%) 0.70s 0.72s
Check Time 4.91s (± 0.43%) 4.88s (± 0.39%) -0.04s (- 0.71%) 4.83s 4.92s
Emit Time 2.85s (± 0.65%) 2.85s (± 0.70%) +0.00s (+ 0.18%) 2.82s 2.90s
Total Time 10.00s (± 0.45%) 9.96s (± 0.33%) -0.04s (- 0.38%) 9.89s 10.02s
TFS - node (v12.1.0, x64)
Memory used 292,357k (± 0.03%) 292,340k (± 0.02%) -16k (- 0.01%) 292,260k 292,552k
Parse Time 1.22s (± 0.62%) 1.22s (± 0.53%) +0.00s (+ 0.16%) 1.20s 1.23s
Bind Time 0.65s (± 0.75%) 0.65s (± 1.35%) -0.00s (- 0.15%) 0.63s 0.68s
Check Time 4.50s (± 0.53%) 4.50s (± 0.70%) -0.00s (- 0.02%) 4.44s 4.59s
Emit Time 2.93s (± 1.52%) 2.94s (± 0.70%) +0.01s (+ 0.24%) 2.89s 2.98s
Total Time 9.30s (± 0.53%) 9.31s (± 0.49%) +0.01s (+ 0.09%) 9.17s 9.40s
material-ui - node (v12.1.0, x64)
Memory used 472,634k (± 0.05%) 472,811k (± 0.01%) +178k (+ 0.04%) 472,715k 472,947k
Parse Time 2.00s (± 0.58%) 2.00s (± 0.76%) -0.00s (- 0.00%) 1.96s 2.03s
Bind Time 0.64s (± 0.69%) 0.64s (± 1.18%) +0.00s (+ 0.31%) 0.62s 0.65s
Check Time 12.51s (± 0.72%) 12.45s (± 0.81%) -0.06s (- 0.46%) 12.33s 12.83s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.15s (± 0.64%) 15.09s (± 0.70%) -0.06s (- 0.37%) 14.94s 15.48s
Angular - node (v14.15.1, x64)
Memory used 321,841k (± 0.01%) 321,831k (± 0.01%) -10k (- 0.00%) 321,794k 321,879k
Parse Time 1.90s (± 0.39%) 1.90s (± 0.61%) +0.00s (+ 0.11%) 1.88s 1.93s
Bind Time 0.85s (± 0.88%) 0.84s (± 0.79%) -0.00s (- 0.47%) 0.83s 0.86s
Check Time 4.83s (± 0.21%) 4.85s (± 0.47%) +0.02s (+ 0.44%) 4.81s 4.92s
Emit Time 5.48s (± 0.67%) 5.47s (± 0.43%) -0.02s (- 0.29%) 5.42s 5.53s
Total Time 13.06s (± 0.33%) 13.07s (± 0.21%) +0.01s (+ 0.08%) 13.00s 13.12s
Compiler-Unions - node (v14.15.1, x64)
Memory used 200,912k (± 0.60%) 200,781k (± 0.55%) -131k (- 0.07%) 199,058k 202,893k
Parse Time 0.80s (± 0.60%) 0.80s (± 1.13%) +0.00s (+ 0.25%) 0.78s 0.82s
Bind Time 0.53s (± 0.76%) 0.53s (± 0.63%) +0.00s (+ 0.19%) 0.52s 0.54s
Check Time 9.67s (± 0.33%) 9.65s (± 0.45%) -0.02s (- 0.24%) 9.56s 9.75s
Emit Time 2.34s (± 0.88%) 2.33s (± 1.09%) -0.00s (- 0.04%) 2.28s 2.40s
Total Time 13.33s (± 0.31%) 13.31s (± 0.38%) -0.02s (- 0.18%) 13.21s 13.43s
Monaco - node (v14.15.1, x64)
Memory used 336,871k (± 0.01%) 336,866k (± 0.01%) -5k (- 0.00%) 336,827k 336,937k
Parse Time 1.55s (± 0.67%) 1.56s (± 0.71%) +0.01s (+ 0.52%) 1.54s 1.59s
Bind Time 0.73s (± 0.81%) 0.73s (± 0.55%) -0.00s (- 0.41%) 0.72s 0.74s
Check Time 4.86s (± 0.43%) 4.85s (± 0.55%) -0.02s (- 0.35%) 4.78s 4.90s
Emit Time 2.91s (± 0.80%) 2.92s (± 0.63%) +0.02s (+ 0.55%) 2.89s 2.98s
Total Time 10.06s (± 0.33%) 10.06s (± 0.33%) +0.00s (+ 0.05%) 9.97s 10.11s
TFS - node (v14.15.1, x64)
Memory used 291,526k (± 0.01%) 291,536k (± 0.01%) +10k (+ 0.00%) 291,497k 291,623k
Parse Time 1.25s (± 1.13%) 1.24s (± 0.78%) -0.01s (- 0.96%) 1.22s 1.27s
Bind Time 0.69s (± 0.98%) 0.69s (± 0.84%) -0.00s (- 0.14%) 0.68s 0.71s
Check Time 4.49s (± 0.43%) 4.49s (± 0.55%) +0.00s (+ 0.09%) 4.44s 4.53s
Emit Time 3.06s (± 0.89%) 3.05s (± 0.52%) -0.01s (- 0.46%) 3.01s 3.09s
Total Time 9.50s (± 0.37%) 9.47s (± 0.36%) -0.03s (- 0.27%) 9.39s 9.54s
material-ui - node (v14.15.1, x64)
Memory used 471,374k (± 0.06%) 471,372k (± 0.06%) -2k (- 0.00%) 470,254k 471,533k
Parse Time 2.06s (± 0.81%) 2.05s (± 0.78%) -0.01s (- 0.29%) 2.02s 2.08s
Bind Time 0.70s (± 0.92%) 0.70s (± 0.64%) -0.00s (- 0.57%) 0.69s 0.71s
Check Time 12.58s (± 0.84%) 12.60s (± 0.72%) +0.01s (+ 0.10%) 12.43s 12.83s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.34s (± 0.70%) 15.35s (± 0.55%) +0.01s (+ 0.04%) 15.21s 15.60s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-198-generic
Architecturex64
Available Memory16 GB
Available Memory8 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
Benchmark Name Iterations
Current 42231 10
Baseline master 10

@typescript-bot typescript-bot mentioned this pull request Feb 9, 2021
Open
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

PR Backlog automation moved this from Waiting on author to Needs merge Feb 9, 2021
@orta
Copy link
Contributor Author

orta commented Feb 9, 2021

I'm heading out for the night, not sure if getting Anders' sign off is a blocker, if it's not can one of you two merge?

@DanielRosenwasser DanielRosenwasser merged commit 8c5fa5c into microsoft:master Feb 9, 2021
PR Backlog automation moved this from Needs merge to Done Feb 9, 2021
@DanielRosenwasser
Copy link
Member

Thanks @orta!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRosenwasser DanielRosenwasser DanielRosenwasser approved these changes

@RyanCavanaugh RyanCavanaugh RyanCavanaugh approved these changes

@weswigham weswigham Awaiting requested review from weswigham

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

Assignees

@orta orta

Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
PR Backlog
  
Done
Milestone
TypeScript 4.2.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@orta @weswigham @typescript-bot @DanielRosenwasser @RyanCavanaugh