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 "Fix contextual discrimination for omitted members (#43937)" #48426

Merged
merged 1 commit into from Mar 25, 2022
Merged

Revert "Fix contextual discrimination for omitted members (#43937)" #48426

merged 1 commit into from Mar 25, 2022

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Mar 25, 2022 •
edited

This reverts commit 751c114, which caused check time for our MUI benchmark to increase by ~25%.

See #48298

@amcasey
Revert "Fix contextual discrimination for omitted members (microsoft#…
07a6845 
…43937)"

This reverts commit 751c114, which caused check time for our MUI benchmark to increase by ~25%.
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 25, 2022
@amcasey amcasey requested a review from weswigham March 25, 2022 16:54
@amcasey
Copy link
Member Author

amcasey commented Mar 25, 2022

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 25, 2022 •
edited

Heya @amcasey, I've started to run the perf test suite on this PR at 07a6845. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..48426

Metric main 48426 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 356,438k (± 0.02%) 356,677k (± 0.02%) +238k (+ 0.07%) 356,533k 356,817k
Parse Time 2.05s (± 0.32%) 2.07s (± 0.67%) +0.01s (+ 0.68%) 2.03s 2.10s
Bind Time 0.86s (± 0.58%) 0.87s (± 1.10%) +0.00s (+ 0.12%) 0.85s 0.90s
Check Time 5.80s (± 0.43%) 5.78s (± 0.61%) -0.02s (- 0.38%) 5.73s 5.85s
Emit Time 5.99s (± 0.79%) 5.99s (± 0.70%) +0.00s (+ 0.05%) 5.91s 6.10s
Total Time 14.71s (± 0.47%) 14.71s (± 0.54%) -0.01s (- 0.03%) 14.55s 14.89s
Compiler-Unions - node (v10.16.3, x64)
Memory used 205,759k (± 0.04%) 205,744k (± 0.03%) -14k (- 0.01%) 205,621k 205,847k
Parse Time 0.85s (± 0.90%) 0.85s (± 0.66%) +0.00s (+ 0.24%) 0.84s 0.86s
Bind Time 0.51s (± 0.71%) 0.52s (± 1.00%) +0.00s (+ 0.97%) 0.51s 0.53s
Check Time 7.94s (± 0.67%) 7.96s (± 0.67%) +0.02s (+ 0.23%) 7.86s 8.11s
Emit Time 2.48s (± 0.86%) 2.53s (± 1.07%) +0.05s (+ 1.93%) 2.50s 2.63s
Total Time 11.78s (± 0.58%) 11.86s (± 0.63%) +0.07s (+ 0.60%) 11.76s 12.11s
Monaco - node (v10.16.3, x64)
Memory used 343,622k (± 0.02%) 343,663k (± 0.01%) +40k (+ 0.01%) 343,515k 343,755k
Parse Time 1.57s (± 0.58%) 1.57s (± 0.77%) +0.00s (+ 0.19%) 1.55s 1.60s
Bind Time 0.75s (± 0.59%) 0.75s (± 0.48%) +0.00s (+ 0.27%) 0.75s 0.76s
Check Time 5.76s (± 0.44%) 5.76s (± 0.61%) +0.00s (+ 0.07%) 5.67s 5.81s
Emit Time 3.22s (± 0.48%) 3.23s (± 0.23%) +0.02s (+ 0.50%) 3.22s 3.25s
Total Time 11.30s (± 0.32%) 11.32s (± 0.40%) +0.02s (+ 0.20%) 11.21s 11.38s
TFS - node (v10.16.3, x64)
Memory used 305,355k (± 0.02%) 305,302k (± 0.02%) -54k (- 0.02%) 305,078k 305,431k
Parse Time 1.28s (± 0.51%) 1.28s (± 0.27%) -0.01s (- 0.39%) 1.27s 1.28s
Bind Time 0.71s (± 1.05%) 0.71s (± 0.83%) +0.00s (+ 0.14%) 0.70s 0.73s
Check Time 5.27s (± 0.53%) 5.23s (± 0.42%) -0.03s (- 0.61%) 5.18s 5.28s
Emit Time 3.45s (± 0.78%) 3.43s (± 0.82%) -0.01s (- 0.41%) 3.36s 3.48s
Total Time 10.71s (± 0.39%) 10.66s (± 0.40%) -0.05s (- 0.45%) 10.58s 10.76s
material-ui - node (v10.16.3, x64)
Memory used 476,213k (± 0.01%) 469,709k (± 0.01%) -6,504k (- 1.37%) 469,537k 469,848k
Parse Time 1.81s (± 0.48%) 1.81s (± 0.50%) -0.00s (- 0.17%) 1.79s 1.83s
Bind Time 0.68s (± 0.91%) 0.68s (± 0.85%) +0.00s (+ 0.15%) 0.67s 0.69s
Check Time 23.05s (± 1.16%) 14.20s (± 0.41%) 🟩-8.85s (-38.38%) 14.06s 14.38s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 25.54s (± 1.07%) 16.69s (± 0.40%) 🟩-8.85s (-34.64%) 16.53s 16.89s
xstate - node (v10.16.3, x64)
Memory used 570,878k (± 0.02%) 574,006k (± 1.24%) +3,129k (+ 0.55%) 570,655k 602,839k
Parse Time 2.57s (± 0.32%) 2.58s (± 0.34%) +0.00s (+ 0.16%) 2.56s 2.60s
Bind Time 1.01s (± 0.49%) 1.00s (± 0.68%) -0.00s (- 0.10%) 0.99s 1.02s
Check Time 1.53s (± 0.53%) 1.53s (± 0.50%) -0.00s (- 0.07%) 1.51s 1.54s
Emit Time 0.07s (± 3.14%) 0.07s (± 0.00%) -0.00s (- 1.41%) 0.07s 0.07s
Total Time 5.18s (± 0.23%) 5.18s (± 0.20%) +0.00s (+ 0.06%) 5.16s 5.20s
Angular - node (v12.1.0, x64)
Memory used 334,173k (± 0.02%) 334,488k (± 0.02%) +315k (+ 0.09%) 334,386k 334,597k
Parse Time 2.04s (± 0.78%) 2.04s (± 0.41%) +0.00s (+ 0.10%) 2.02s 2.06s
Bind Time 0.83s (± 1.16%) 0.83s (± 0.57%) -0.00s (- 0.12%) 0.82s 0.84s
Check Time 5.63s (± 0.34%) 5.62s (± 0.31%) -0.01s (- 0.18%) 5.58s 5.65s
Emit Time 6.22s (± 0.73%) 6.31s (± 1.03%) +0.09s (+ 1.49%) 6.20s 6.50s
Total Time 14.73s (± 0.35%) 14.81s (± 0.47%) +0.08s (+ 0.56%) 14.65s 14.99s
Compiler-Unions - node (v12.1.0, x64)
Memory used 193,180k (± 0.13%) 193,202k (± 0.13%) +22k (+ 0.01%) 192,627k 193,583k
Parse Time 0.84s (± 0.53%) 0.84s (± 1.10%) +0.00s (+ 0.00%) 0.82s 0.86s
Bind Time 0.53s (± 1.09%) 0.53s (± 0.98%) -0.00s (- 0.38%) 0.52s 0.54s
Check Time 7.44s (± 0.47%) 7.44s (± 0.65%) +0.00s (+ 0.04%) 7.36s 7.59s
Emit Time 2.54s (± 0.93%) 2.55s (± 1.16%) +0.01s (+ 0.43%) 2.50s 2.64s
Total Time 11.36s (± 0.42%) 11.37s (± 0.50%) +0.01s (+ 0.07%) 11.26s 11.53s
Monaco - node (v12.1.0, x64)
Memory used 326,580k (± 0.06%) 326,614k (± 0.02%) +34k (+ 0.01%) 326,500k 326,756k
Parse Time 1.54s (± 0.69%) 1.55s (± 0.45%) +0.01s (+ 0.78%) 1.53s 1.56s
Bind Time 0.74s (± 0.83%) 0.75s (± 0.80%) +0.00s (+ 0.54%) 0.74s 0.76s
Check Time 5.60s (± 0.68%) 5.60s (± 0.56%) -0.00s (- 0.07%) 5.54s 5.67s
Emit Time 3.28s (± 1.18%) 3.25s (± 0.64%) -0.03s (- 0.82%) 3.21s 3.31s
Total Time 11.16s (± 0.57%) 11.14s (± 0.49%) -0.02s (- 0.14%) 11.05s 11.28s
TFS - node (v12.1.0, x64)
Memory used 289,930k (± 0.02%) 289,922k (± 0.02%) -8k (- 0.00%) 289,783k 290,044k
Parse Time 1.29s (± 0.86%) 1.30s (± 0.89%) +0.01s (+ 0.78%) 1.27s 1.32s
Bind Time 0.71s (± 0.71%) 0.70s (± 0.74%) -0.01s (- 0.85%) 0.69s 0.71s
Check Time 5.16s (± 0.41%) 5.14s (± 0.47%) -0.02s (- 0.31%) 5.08s 5.20s
Emit Time 3.47s (± 0.53%) 3.49s (± 0.61%) +0.03s (+ 0.75%) 3.44s 3.53s
Total Time 10.61s (± 0.37%) 10.63s (± 0.41%) +0.01s (+ 0.14%) 10.51s 10.71s
material-ui - node (v12.1.0, x64)
Memory used 454,872k (± 0.10%) 448,640k (± 0.06%) -6,232k (- 1.37%) 447,557k 448,866k
Parse Time 1.81s (± 0.71%) 1.81s (± 0.50%) +0.00s (+ 0.06%) 1.79s 1.82s
Bind Time 0.64s (± 0.53%) 0.64s (± 0.77%) +0.00s (+ 0.31%) 0.64s 0.66s
Check Time 20.37s (± 1.08%) 12.89s (± 0.58%) 🟩-7.48s (-36.73%) 12.73s 13.10s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 22.82s (± 0.93%) 15.34s (± 0.48%) 🟩-7.48s (-32.77%) 15.17s 15.55s
xstate - node (v12.1.0, x64)
Memory used 536,408k (± 0.02%) 536,355k (± 0.01%) -54k (- 0.01%) 536,174k 536,543k
Parse Time 2.52s (± 0.36%) 2.53s (± 0.44%) +0.01s (+ 0.28%) 2.50s 2.55s
Bind Time 1.03s (± 0.56%) 1.03s (± 0.88%) -0.00s (- 0.19%) 1.01s 1.05s
Check Time 1.48s (± 0.75%) 1.47s (± 0.64%) -0.01s (- 0.41%) 1.45s 1.49s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.10s (± 0.29%) 5.10s (± 0.33%) +0.00s (+ 0.02%) 5.06s 5.13s
Angular - node (v14.15.1, x64)
Memory used 332,480k (± 0.01%) 332,719k (± 0.00%) +238k (+ 0.07%) 332,691k 332,746k
Parse Time 2.02s (± 0.62%) 2.03s (± 0.57%) +0.01s (+ 0.35%) 2.01s 2.06s
Bind Time 0.87s (± 0.75%) 0.87s (± 0.46%) +0.00s (+ 0.12%) 0.86s 0.88s
Check Time 5.63s (± 0.28%) 5.63s (± 0.54%) -0.00s (- 0.09%) 5.57s 5.72s
Emit Time 6.30s (± 0.66%) 6.29s (± 0.77%) -0.01s (- 0.22%) 6.21s 6.42s
Total Time 14.83s (± 0.36%) 14.82s (± 0.45%) -0.01s (- 0.09%) 14.69s 14.96s
Compiler-Unions - node (v14.15.1, x64)
Memory used 194,305k (± 0.56%) 194,615k (± 0.48%) +310k (+ 0.16%) 192,086k 195,316k
Parse Time 0.85s (± 0.58%) 0.86s (± 0.61%) +0.01s (+ 0.59%) 0.85s 0.87s
Bind Time 0.56s (± 0.40%) 0.56s (± 0.71%) 0.00s ( 0.00%) 0.55s 0.57s
Check Time 7.54s (± 0.59%) 7.53s (± 0.47%) -0.01s (- 0.15%) 7.46s 7.63s
Emit Time 2.49s (± 0.75%) 2.51s (± 0.69%) +0.02s (+ 0.96%) 2.48s 2.56s
Total Time 11.45s (± 0.43%) 11.46s (± 0.43%) +0.02s (+ 0.13%) 11.38s 11.63s
Monaco - node (v14.15.1, x64)
Memory used 325,395k (± 0.01%) 325,409k (± 0.01%) +13k (+ 0.00%) 325,360k 325,463k
Parse Time 1.57s (± 0.71%) 1.58s (± 0.84%) +0.00s (+ 0.25%) 1.56s 1.61s
Bind Time 0.78s (± 0.64%) 0.78s (± 0.48%) 0.00s ( 0.00%) 0.77s 0.78s
Check Time 5.51s (± 0.53%) 5.50s (± 0.48%) -0.01s (- 0.18%) 5.44s 5.56s
Emit Time 3.29s (± 0.60%) 3.31s (± 0.80%) +0.02s (+ 0.58%) 3.26s 3.38s
Total Time 11.16s (± 0.31%) 11.17s (± 0.40%) +0.01s (+ 0.12%) 11.09s 11.24s
TFS - node (v14.15.1, x64)
Memory used 288,907k (± 0.01%) 288,896k (± 0.01%) -10k (- 0.00%) 288,841k 288,954k
Parse Time 1.31s (± 1.14%) 1.30s (± 0.88%) -0.00s (- 0.31%) 1.29s 1.34s
Bind Time 0.74s (± 0.67%) 0.76s (± 4.12%) +0.02s (+ 2.85%) 0.73s 0.84s
Check Time 5.16s (± 0.67%) 5.18s (± 0.50%) +0.02s (+ 0.41%) 5.13s 5.24s
Emit Time 3.52s (± 1.43%) 3.53s (± 1.96%) +0.01s (+ 0.17%) 3.37s 3.64s
Total Time 10.73s (± 0.39%) 10.77s (± 0.78%) +0.04s (+ 0.36%) 10.51s 10.94s
material-ui - node (v14.15.1, x64)
Memory used 453,546k (± 0.05%) 447,054k (± 0.01%) -6,492k (- 1.43%) 447,004k 447,119k
Parse Time 1.86s (± 0.67%) 1.85s (± 0.51%) -0.00s (- 0.11%) 1.84s 1.88s
Bind Time 0.70s (± 1.04%) 0.70s (± 1.08%) +0.00s (+ 0.28%) 0.69s 0.72s
Check Time 20.41s (± 0.70%) 13.03s (± 0.47%) 🟩-7.38s (-36.16%) 12.86s 13.14s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 22.97s (± 0.59%) 15.58s (± 0.41%) 🟩-7.38s (-32.14%) 15.40s 15.69s
xstate - node (v14.15.1, x64)
Memory used 534,094k (± 0.01%) 534,101k (± 0.00%) +7k (+ 0.00%) 534,069k 534,133k
Parse Time 2.56s (± 0.43%) 2.58s (± 0.48%) +0.01s (+ 0.47%) 2.55s 2.60s
Bind Time 1.14s (± 0.84%) 1.14s (± 0.64%) +0.00s (+ 0.18%) 1.12s 1.15s
Check Time 1.51s (± 0.70%) 1.51s (± 0.41%) +0.00s (+ 0.07%) 1.50s 1.52s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.29s (± 0.32%) 5.30s (± 0.22%) +0.02s (+ 0.36%) 5.29s 5.34s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory4 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)
  • xstate - node (v10.16.3, x64)
  • xstate - node (v12.1.0, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48426 10
Baseline main 10

Developer Information:

Download Benchmark

@amcasey
Copy link
Member Author

amcasey commented Mar 25, 2022

Seeing a 35% reduction in check time for MUI.

weswigham
weswigham approved these changes Mar 25, 2022
View reviewed changes
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

@erikbrinkman we can look into re-merging this once we've solved the perf issue, we just don't wanna ship such a big perf regression in the next beta.

@amcasey amcasey merged commit e742625 into microsoft:main Mar 25, 2022
@amcasey amcasey deleted the RevertMuiRegression branch March 25, 2022 20:09
@erikbrinkman erikbrinkman mentioned this pull request Mar 27, 2022
Closed
sidharthv96 added a commit to sidharthv96/TypeScript that referenced this pull request Apr 1, 2022
@sidharthv96
Merge remote-tracking branch 'upstream/main' into fix/46149
e149cc3 
* upstream/main: (473 commits)
  Correct node used for isDefinition calculation (microsoft#48499)
  fix(48405): emit dummy members from a mapped type (microsoft#48481)
  CFA for dependent parameters typed by generic constraints (microsoft#48411)
  No contextual typing from return types for boolean literals (microsoft#48380)
  fix(47733): omit JSDoc comment template suggestion on node with existing JSDoc (microsoft#47748)
  Ensure that we copy empty NodeArrays during transform (microsoft#48490)
  Add a new compiler option `moduleSuffixes` to expand the node module resolver's search algorithm (microsoft#48189)
  feat(27615): Add missing member fix should work for type literals (microsoft#47212)
  Add label details to completion entry (microsoft#48429)
  Enable method signature completion for object literals (microsoft#48168)
  Fix string literal completions when a partially-typed string fixes inference to a type parameter (microsoft#48410)
  fix(48445): show errors on type-only import/export specifiers in JavaScript files (microsoft#48449)
  Fix newline inserted in empty block at end of formatting range (microsoft#48463)
  Prevent looking up symbol for as const from triggering an error (microsoft#48464)
  Revise accessor resolution logic and error reporting (microsoft#48459)
  fix(48166): skip checking module.exports in a truthiness call expression (microsoft#48337)
  LEGO: Merge pull request 48450
  LEGO: Merge pull request 48436
  fix(48031): show circularity error for self referential get accessor annotations (microsoft#48050)
  Revert "Fix contextual discrimination for omitted members (microsoft#43937)" (microsoft#48426)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weswigham weswigham weswigham approved these changes

@DanielRosenwasser DanielRosenwasser Awaiting requested review from DanielRosenwasser

Assignees

@amcasey amcasey

Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@amcasey @typescript-bot @weswigham