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.

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

Only normalize intersections that include {} #50535

Merged
merged 2 commits into from Aug 31, 2022
Merged

Only normalize intersections that include {} #50535

merged 2 commits into from Aug 31, 2022

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Aug 30, 2022

Fixes #50515.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 30, 2022
@ahejlsberg
Copy link
Member Author

ahejlsberg commented Aug 30, 2022

@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 Aug 30, 2022 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022

Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022

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

Here they are:

Comparison Report - main..50535

Metric main 50535 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 338,727k (± 0.00%) 338,739k (± 0.01%) +12k (+ 0.00%) 338,694k 338,796k
Parse Time 2.08s (± 0.70%) 2.07s (± 0.60%) -0.01s (- 0.34%) 2.05s 2.11s
Bind Time 0.80s (± 0.62%) 0.79s (± 0.28%) -0.00s (- 0.63%) 0.79s 0.80s
Check Time 5.82s (± 0.58%) 5.82s (± 0.39%) -0.01s (- 0.10%) 5.78s 5.88s
Emit Time 6.18s (± 0.49%) 6.15s (± 0.59%) -0.03s (- 0.53%) 6.05s 6.22s
Total Time 14.87s (± 0.27%) 14.82s (± 0.26%) -0.05s (- 0.32%) 14.74s 14.90s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,654k (± 0.02%) 192,634k (± 0.02%) -20k (- 0.01%) 192,555k 192,686k
Parse Time 0.86s (± 0.52%) 0.85s (± 1.04%) -0.00s (- 0.47%) 0.84s 0.88s
Bind Time 0.49s (± 0.75%) 0.48s (± 0.70%) -0.00s (- 0.62%) 0.48s 0.49s
Check Time 6.70s (± 0.55%) 6.68s (± 0.46%) -0.02s (- 0.30%) 6.63s 6.77s
Emit Time 2.41s (± 1.02%) 2.39s (± 0.74%) -0.02s (- 0.83%) 2.36s 2.43s
Total Time 10.46s (± 0.45%) 10.41s (± 0.36%) -0.05s (- 0.47%) 10.33s 10.48s
Monaco - node (v14.15.1, x64)
Memory used 326,505k (± 0.01%) 326,510k (± 0.01%) +4k (+ 0.00%) 326,457k 326,554k
Parse Time 1.57s (± 0.59%) 1.57s (± 0.88%) +0.00s (+ 0.13%) 1.55s 1.61s
Bind Time 0.73s (± 0.85%) 0.72s (± 0.94%) -0.01s (- 0.69%) 0.71s 0.74s
Check Time 5.73s (± 0.51%) 5.69s (± 0.44%) -0.04s (- 0.66%) 5.64s 5.74s
Emit Time 3.35s (± 0.61%) 3.31s (± 0.67%) -0.04s (- 1.31%) 3.26s 3.35s
Total Time 11.39s (± 0.42%) 11.30s (± 0.41%) -0.09s (- 0.77%) 11.20s 11.40s
TFS - node (v14.15.1, x64)
Memory used 289,645k (± 0.00%) 289,632k (± 0.01%) -13k (- 0.00%) 289,597k 289,683k
Parse Time 1.31s (± 0.83%) 1.31s (± 1.25%) -0.00s (- 0.23%) 1.25s 1.33s
Bind Time 0.78s (± 2.29%) 0.79s (± 1.07%) +0.00s (+ 0.64%) 0.76s 0.80s
Check Time 5.36s (± 0.54%) 5.31s (± 1.08%) -0.05s (- 0.99%) 5.09s 5.37s
Emit Time 3.60s (± 0.77%) 3.54s (± 1.24%) -0.06s (- 1.75%) 3.37s 3.58s
Total Time 11.05s (± 0.38%) 10.94s (± 1.10%) -0.11s (- 1.04%) 10.47s 11.06s
material-ui - node (v14.15.1, x64)
Memory used 438,087k (± 0.06%) 435,894k (± 0.00%) -2,193k (- 0.50%) 435,866k 435,935k
Parse Time 1.86s (± 0.59%) 1.85s (± 0.39%) -0.02s (- 0.81%) 1.84s 1.87s
Bind Time 0.59s (± 0.62%) 0.58s (± 0.59%) -0.01s (- 1.54%) 0.57s 0.58s
Check Time 13.06s (± 0.95%) 12.87s (± 1.34%) -0.19s (- 1.46%) 12.68s 13.54s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.51s (± 0.81%) 15.29s (± 1.11%) -0.21s (- 1.38%) 15.11s 15.96s
xstate - node (v14.15.1, x64)
Memory used 547,388k (± 0.00%) 546,742k (± 0.00%) -646k (- 0.12%) 546,716k 546,774k
Parse Time 2.60s (± 0.45%) 2.59s (± 0.66%) -0.01s (- 0.23%) 2.58s 2.66s
Bind Time 0.97s (± 0.70%) 0.97s (± 1.18%) -0.01s (- 0.62%) 0.96s 1.01s
Check Time 1.54s (± 0.62%) 1.52s (± 0.48%) -0.03s (- 1.68%) 1.50s 1.53s
Emit Time 0.07s (± 4.13%) 0.07s (± 4.66%) +0.00s (+ 1.39%) 0.07s 0.08s
Total Time 5.20s (± 0.42%) 5.15s (± 0.44%) -0.05s (- 0.92%) 5.11s 5.23s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory15 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 50535 10
Baseline main 10

Developer Information:

Download Benchmark

@ahejlsberg ahejlsberg mentioned this pull request Aug 30, 2022
Closed
DanielRosenwasser
DanielRosenwasser reviewed Aug 30, 2022
View changes
tests/baselines/reference/conditionalTypes2.errors.txt
@@ -32,7 +32,7 @@ tests/cases/conformance/types/conditional/conditionalTypes2.ts(74,12): error TS2
Property 'bat' is missing in type 'Foo & Bar' but required in type '{ foo: string; bat: string; }'.
tests/cases/conformance/types/conditional/conditionalTypes2.ts(75,12): error TS2345: Argument of type 'Extract2<T, Foo, Bar>' is not assignable to parameter of type '{ foo: string; bat: string; }'.
Type 'T extends Bar ? T : never' is not assignable to type '{ foo: string; bat: string; }'.
Type 'Bar & Foo & T' is not assignable to type '{ foo: string; bat: string; }'.
Property 'bat' is missing in type 'Bar & Foo' but required in type '{ foo: string; bat: string; }'.
Copy link
Member

@DanielRosenwasser DanielRosenwasser Aug 30, 2022

Choose a reason for hiding this comment

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

This is really the only test that's affected? We don't have any other tests that try to intersect a type with {}?

Copy link
Member Author

@ahejlsberg ahejlsberg Aug 30, 2022 •
edited

Choose a reason for hiding this comment

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

We have plenty of those, but what you're seeing here is the difference between normalizing all intersections in relationship checking vs. just normalizing intersections containing {}. Previously we didn't normalize intersections at all, relying instead on normalizing each individual constituent as we broke down the intersection in relationship checking. That worked when NonNullable<T> was a conditional type, but not with the new representation.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 30, 2022

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

cc @andrewbranch

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

Heya @RyanCavanaugh, I've started to run the task to cherry-pick this into release-4.8 on this PR at 698c3ab. You can monitor the build here.

@typescript-bot typescript-bot mentioned this pull request Aug 30, 2022
Merged
@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022

Hey @RyanCavanaugh, I've opened #50549 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Aug 30, 2022
@ahejlsberg
Cherry-pick PR microsoft#50535 into release-4.8
228169a 
Component commits:
7bd2a6e Only normalize intersections that include {}

698c3ab Accept new baselines
@ahejlsberg
Copy link
Member Author

ahejlsberg commented Aug 30, 2022

@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 30, 2022 •
edited

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

Update: The results are in!

@ahejlsberg ahejlsberg mentioned this pull request Aug 30, 2022
Merged
@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 31, 2022

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

Everything looks good!

@ahejlsberg
Copy link
Member Author

ahejlsberg commented Aug 31, 2022

Tests are all clean and perf looks good. This one is ready to merge.

@andrewbranch andrewbranch added this to the TypeScript 4.8.3 milestone Aug 31, 2022
@typescript-bot typescript-bot removed the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 31, 2022
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 31, 2022
@ahejlsberg ahejlsberg merged commit 43f8ae6 into main Aug 31, 2022
13 checks passed
@ahejlsberg ahejlsberg deleted the fix50515 branch Aug 31, 2022
andrewbranch pushed a commit that referenced this pull request Sep 1, 2022
@typescript-bot @ahejlsberg
Cherry-pick PR #50535 into release-4.8 (#50549)
ac0cc8c 
Component commits:
7bd2a6e Only normalize intersections that include {}

698c3ab Accept new baselines

Co-authored-by: Anders Hejlsberg <andersh@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRosenwasser DanielRosenwasser

@weswigham weswigham

@andrewbranch andrewbranch

Assignees

@ahejlsberg ahejlsberg

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
TypeScript 4.8.3
Development

Successfully merging this pull request may close these issues.

Performance issue on updating Babel monorepo typescript from 4.7.4 to 4.8.2
6 participants
@ahejlsberg @typescript-bot @RyanCavanaugh @DanielRosenwasser @weswigham @andrewbranch