Add an early bail when structurally comparing similar types - #42726
Closed
Wesley Wigham (weswigham) wants to merge 3 commits into
Closed
Add an early bail when structurally comparing similar types#42726Wesley Wigham (weswigham) wants to merge 3 commits into
Wesley Wigham (weswigham) wants to merge 3 commits into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
… the middle of a more general comparison of those same types
Wesley Wigham (weswigham)
requested review from
Anders Hejlsberg (ahejlsberg) and
Nathan Shively-Sanders (sandersn)
February 9, 2021 23:50
This was referenced Feb 9, 2021
Copy link
Copy Markdown
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the tarball bundle task on this PR at b3856bd. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the perf test suite on this PR at b3856bd. You can monitor the build here. Update: The results are in! |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the extended test suite on this PR at b3856bd. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the parallelized community code test suite on this PR at b3856bd. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the parallelized Definitely Typed test suite on this PR at b3856bd. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
Sorry, something went wrong.
…adjust logic to match
Copy link
Copy Markdown
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this Had to make a small adjustment to the logic; alias symbols and type references aren't mutually exclusive anymore, so we have to check for type reference-ness first. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the tarball bundle task on this PR at 4a8ef89. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the parallelized Definitely Typed test suite on this PR at 4a8ef89. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the extended test suite on this PR at 4a8ef89. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the perf test suite on this PR at 4a8ef89. You can monitor the build here. Update: The results are in! |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Heya Wesley Wigham (@weswigham), I've started to run the parallelized community code test suite on this PR at 4a8ef89. You can monitor the build here. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Wesley Wigham (@weswigham) Comparison Report - master..42726
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Hey Wesley Wigham (@weswigham), 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/95553/artifacts?artifactName=tgz&fileId=627B70266F13C43643183241D694BE76F87E1DBF94487BC8B8B8A1E2E283C3D302&fileName=/typescript-4.2.0-insiders.20210210.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-42726-14".; |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Wesley Wigham (@weswigham) Comparison Report - master..42726
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
Unfortunately, we never finished reviewing this PR. It is pretty old now, so I'm going to close it to reduce the number of open PRs. |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Reviewers
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If we are already in the middle of a more general comparison of those same types.
This is an optimization I pulled out of #31633 and tweaked a bit to bit a little more effective.
Fixes #38970
This and #42727 aren't technically mutually exclusive, but given that they target the same underlying inefficiency, I imagine we'll only take one until we have good reason to take the other.