Skip to content

Only normalize intersections that include {} - #50535

Merged
Anders Hejlsberg (ahejlsberg) merged 2 commits into
mainfrom
fix50515
Aug 31, 2022
Merged

Only normalize intersections that include {}#50535
Anders Hejlsberg (ahejlsberg) merged 2 commits into
mainfrom
fix50515

Conversation

@ahejlsberg

Copy link
Copy Markdown
Member

Fixes #50515.

@ahejlsberg

Copy link
Copy Markdown
Member Author

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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
Copy Markdown
Contributor

Anders Hejlsberg (@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
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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
Copy Markdown
Contributor

Anders Hejlsberg (@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

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
Copy Markdown
Member

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 {}?

@ahejlsberg Anders Hejlsberg (ahejlsberg) Aug 30, 2022

Copy link
Copy Markdown
Member Author

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
Copy Markdown
Member

TypeScript Bot (@typescript-bot) cherry-pick this to release-4.8

cc Andrew Branch (@andrewbranch)

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@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

Copy link
Copy Markdown
Contributor

Hey Ryan Cavanaugh (@RyanCavanaugh), I've opened #50549 for you.

@ahejlsberg

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test top100

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@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
Copy Markdown
Contributor

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

Everything looks good!

@ahejlsberg

Copy link
Copy Markdown
Member Author

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

@typescript-bot 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 (typescript-bot) added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 31, 2022
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

Performance issue on updating Babel monorepo typescript from 4.7.4 to 4.8.2

6 participants