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

Always create optional properties when spreading objects conditionally #40778

Merged
merged 6 commits into from
Oct 12, 2020
Merged

Always create optional properties when spreading objects conditionally #40778

merged 6 commits into from
Oct 12, 2020

Conversation

weswigham
Copy link
Member

Fixes #40754

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Sep 25, 2020
@weswigham
Copy link
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 25, 2020 •
edited
Loading

Hey @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/86538/artifacts?artifactName=tgz&fileId=909B0605B6A1C79D492FD1089766D5E2731694154481EA36B033363558CFA19002&fileName=/typescript-4.1.0-insiders.20200925.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..40778

Metric master 40778 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 349,605k (± 0.02%) 349,567k (± 0.02%) -38k (- 0.01%) 349,383k 349,754k
Parse Time 2.02s (± 0.65%) 2.01s (± 0.72%) -0.01s (- 0.40%) 1.98s 2.04s
Bind Time 0.83s (± 0.74%) 0.83s (± 0.96%) +0.00s (+ 0.12%) 0.82s 0.85s
Check Time 4.93s (± 0.54%) 4.92s (± 0.48%) -0.01s (- 0.22%) 4.88s 4.98s
Emit Time 5.23s (± 1.04%) 5.21s (± 0.91%) -0.02s (- 0.36%) 5.13s 5.35s
Total Time 13.01s (± 0.52%) 12.98s (± 0.52%) -0.04s (- 0.27%) 12.87s 13.20s
Monaco - node (v10.16.3, x64)
Memory used 354,382k (± 0.02%) 354,322k (± 0.02%) -60k (- 0.02%) 354,201k 354,466k
Parse Time 1.58s (± 0.57%) 1.56s (± 0.73%) -0.02s (- 1.14%) 1.53s 1.58s
Bind Time 0.71s (± 0.83%) 0.71s (± 0.42%) -0.00s (- 0.28%) 0.71s 0.72s
Check Time 5.07s (± 0.49%) 5.05s (± 0.44%) -0.02s (- 0.47%) 5.00s 5.08s
Emit Time 2.76s (± 0.38%) 2.75s (± 0.51%) -0.01s (- 0.43%) 2.72s 2.78s
Total Time 10.13s (± 0.34%) 10.07s (± 0.36%) -0.05s (- 0.50%) 9.99s 10.14s
TFS - node (v10.16.3, x64)
Memory used 307,583k (± 0.02%) 307,568k (± 0.02%) -15k (- 0.00%) 307,458k 307,672k
Parse Time 1.23s (± 0.73%) 1.22s (± 0.46%) -0.01s (- 0.65%) 1.21s 1.23s
Bind Time 0.68s (± 0.54%) 0.67s (± 1.12%) -0.01s (- 1.33%) 0.65s 0.68s
Check Time 4.52s (± 0.51%) 4.53s (± 0.59%) +0.00s (+ 0.09%) 4.48s 4.59s
Emit Time 2.90s (± 1.21%) 2.92s (± 0.88%) +0.02s (+ 0.83%) 2.84s 2.96s
Total Time 9.32s (± 0.62%) 9.33s (± 0.39%) +0.01s (+ 0.10%) 9.26s 9.42s
material-ui - node (v10.16.3, x64)
Memory used 488,920k (± 0.02%) 488,907k (± 0.01%) -12k (- 0.00%) 488,784k 489,007k
Parse Time 1.99s (± 0.46%) 1.98s (± 0.56%) -0.01s (- 0.55%) 1.96s 2.01s
Bind Time 0.65s (± 0.61%) 0.65s (± 0.80%) -0.00s (- 0.31%) 0.64s 0.66s
Check Time 13.64s (± 0.93%) 13.50s (± 0.65%) -0.14s (- 1.00%) 13.32s 13.66s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.28s (± 0.79%) 16.12s (± 0.55%) -0.15s (- 0.95%) 15.94s 16.31s
Angular - node (v12.1.0, x64)
Memory used 326,821k (± 0.02%) 326,808k (± 0.02%) -13k (- 0.00%) 326,664k 326,938k
Parse Time 1.99s (± 0.68%) 2.00s (± 0.63%) +0.01s (+ 0.40%) 1.97s 2.04s
Bind Time 0.81s (± 0.42%) 0.81s (± 0.42%) +0.01s (+ 0.74%) 0.81s 0.82s
Check Time 4.79s (± 0.46%) 4.83s (± 0.56%) +0.03s (+ 0.67%) 4.78s 4.90s
Emit Time 5.36s (± 0.57%) 5.37s (± 0.62%) +0.00s (+ 0.07%) 5.28s 5.43s
Total Time 12.95s (± 0.41%) 13.00s (± 0.48%) +0.05s (+ 0.37%) 12.86s 13.15s
Monaco - node (v12.1.0, x64)
Memory used 336,542k (± 0.02%) 336,534k (± 0.02%) -8k (- 0.00%) 336,421k 336,733k
Parse Time 1.55s (± 0.57%) 1.54s (± 0.65%) -0.01s (- 0.52%) 1.52s 1.57s
Bind Time 0.69s (± 0.43%) 0.69s (± 0.72%) +0.00s (+ 0.43%) 0.69s 0.71s
Check Time 4.87s (± 0.36%) 4.86s (± 0.48%) -0.01s (- 0.23%) 4.81s 4.93s
Emit Time 2.81s (± 1.06%) 2.82s (± 0.49%) +0.01s (+ 0.18%) 2.78s 2.84s
Total Time 9.93s (± 0.47%) 9.92s (± 0.23%) -0.01s (- 0.12%) 9.87s 9.97s
TFS - node (v12.1.0, x64)
Memory used 291,803k (± 0.02%) 291,836k (± 0.02%) +34k (+ 0.01%) 291,710k 291,973k
Parse Time 1.23s (± 0.67%) 1.24s (± 1.15%) +0.01s (+ 0.41%) 1.21s 1.27s
Bind Time 0.64s (± 0.53%) 0.64s (± 0.73%) -0.00s (- 0.47%) 0.63s 0.65s
Check Time 4.45s (± 0.40%) 4.43s (± 0.44%) -0.01s (- 0.29%) 4.38s 4.48s
Emit Time 2.93s (± 1.63%) 2.93s (± 1.17%) +0.00s (+ 0.03%) 2.87s 3.03s
Total Time 9.25s (± 0.60%) 9.24s (± 0.48%) -0.01s (- 0.08%) 9.19s 9.37s
material-ui - node (v12.1.0, x64)
Memory used 466,836k (± 0.08%) 466,862k (± 0.07%) +26k (+ 0.01%) 465,962k 467,204k
Parse Time 2.01s (± 0.57%) 2.02s (± 0.54%) +0.01s (+ 0.40%) 2.00s 2.04s
Bind Time 0.64s (± 1.38%) 0.64s (± 0.87%) -0.00s (- 0.31%) 0.63s 0.65s
Check Time 12.19s (± 1.06%) 12.16s (± 1.18%) -0.03s (- 0.25%) 11.95s 12.57s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.85s (± 0.91%) 14.82s (± 1.01%) -0.03s (- 0.17%) 14.59s 15.24s
Angular - node (v8.9.0, x64)
Memory used 346,275k (± 0.03%) 346,308k (± 0.03%) +33k (+ 0.01%) 346,051k 346,506k
Parse Time 2.55s (± 0.54%) 2.54s (± 0.41%) -0.00s (- 0.12%) 2.52s 2.57s
Bind Time 0.87s (± 0.94%) 0.86s (± 0.75%) -0.01s (- 0.92%) 0.85s 0.88s
Check Time 5.56s (± 0.81%) 5.55s (± 0.73%) -0.01s (- 0.22%) 5.46s 5.62s
Emit Time 6.04s (± 1.55%) 6.11s (± 1.95%) +0.07s (+ 1.21%) 5.88s 6.37s
Total Time 15.01s (± 0.34%) 15.06s (± 0.81%) +0.05s (+ 0.33%) 14.78s 15.30s
Monaco - node (v8.9.0, x64)
Memory used 355,603k (± 0.01%) 355,641k (± 0.02%) +38k (+ 0.01%) 355,425k 355,753k
Parse Time 1.88s (± 0.45%) 1.88s (± 0.47%) +0.00s (+ 0.05%) 1.86s 1.90s
Bind Time 0.90s (± 0.98%) 0.89s (± 0.55%) -0.00s (- 0.33%) 0.88s 0.90s
Check Time 5.62s (± 0.39%) 5.60s (± 0.44%) -0.02s (- 0.37%) 5.54s 5.66s
Emit Time 3.29s (± 1.37%) 3.30s (± 1.19%) +0.01s (+ 0.27%) 3.20s 3.37s
Total Time 11.69s (± 0.57%) 11.68s (± 0.36%) -0.01s (- 0.13%) 11.54s 11.73s
TFS - node (v8.9.0, x64)
Memory used 309,293k (± 0.01%) 309,285k (± 0.02%) -7k (- 0.00%) 309,131k 309,379k
Parse Time 1.55s (± 0.43%) 1.55s (± 0.50%) -0.00s (- 0.26%) 1.53s 1.56s
Bind Time 0.68s (± 0.77%) 0.68s (± 1.07%) -0.00s (- 0.15%) 0.66s 0.69s
Check Time 5.31s (± 0.64%) 5.29s (± 0.37%) -0.02s (- 0.36%) 5.24s 5.32s
Emit Time 2.95s (± 0.81%) 2.95s (± 0.63%) -0.00s (- 0.03%) 2.91s 2.99s
Total Time 10.48s (± 0.42%) 10.46s (± 0.34%) -0.03s (- 0.25%) 10.37s 10.51s
material-ui - node (v8.9.0, x64)
Memory used 493,358k (± 0.01%) 493,364k (± 0.03%) +6k (+ 0.00%) 493,162k 493,810k
Parse Time 2.41s (± 0.54%) 2.41s (± 0.52%) -0.00s (- 0.12%) 2.38s 2.43s
Bind Time 0.82s (± 1.22%) 0.81s (± 1.26%) -0.01s (- 0.73%) 0.79s 0.84s
Check Time 17.89s (± 1.22%) 17.93s (± 0.87%) +0.05s (+ 0.25%) 17.50s 18.19s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.11s (± 1.01%) 21.15s (± 0.77%) +0.04s (+ 0.18%) 20.69s 21.38s
Angular - node (v8.9.0, x86)
Memory used 198,607k (± 0.03%) 198,602k (± 0.03%) -5k (- 0.00%) 198,420k 198,750k
Parse Time 2.48s (± 0.58%) 2.47s (± 1.13%) -0.01s (- 0.44%) 2.44s 2.56s
Bind Time 1.00s (± 0.89%) 1.01s (± 0.72%) +0.00s (+ 0.40%) 0.99s 1.02s
Check Time 5.01s (± 0.43%) 5.01s (± 0.72%) +0.01s (+ 0.14%) 4.93s 5.08s
Emit Time 5.96s (± 0.90%) 5.91s (± 0.84%) -0.05s (- 0.77%) 5.81s 6.02s
Total Time 14.45s (± 0.36%) 14.41s (± 0.61%) -0.04s (- 0.30%) 14.24s 14.59s
Monaco - node (v8.9.0, x86)
Memory used 201,440k (± 0.03%) 201,419k (± 0.01%) -20k (- 0.01%) 201,347k 201,483k
Parse Time 1.93s (± 0.75%) 1.92s (± 0.51%) -0.02s (- 0.83%) 1.90s 1.95s
Bind Time 0.70s (± 0.85%) 0.71s (± 0.67%) +0.00s (+ 0.28%) 0.70s 0.72s
Check Time 5.49s (± 1.46%) 5.46s (± 1.37%) -0.03s (- 0.47%) 5.36s 5.68s
Emit Time 2.98s (± 4.01%) 2.99s (± 3.87%) +0.00s (+ 0.10%) 2.65s 3.10s
Total Time 11.11s (± 0.84%) 11.08s (± 0.53%) -0.03s (- 0.31%) 10.90s 11.20s
TFS - node (v8.9.0, x86)
Memory used 176,778k (± 0.03%) 176,801k (± 0.03%) +24k (+ 0.01%) 176,677k 176,885k
Parse Time 1.58s (± 0.85%) 1.59s (± 0.84%) +0.01s (+ 0.51%) 1.57s 1.63s
Bind Time 0.64s (± 0.53%) 0.65s (± 1.47%) +0.00s (+ 0.47%) 0.63s 0.67s
Check Time 4.80s (± 0.53%) 4.80s (± 0.50%) +0.01s (+ 0.13%) 4.76s 4.85s
Emit Time 2.78s (± 0.59%) 2.79s (± 1.17%) +0.02s (+ 0.54%) 2.73s 2.89s
Total Time 9.80s (± 0.31%) 9.83s (± 0.45%) +0.03s (+ 0.31%) 9.73s 9.94s
material-ui - node (v8.9.0, x86)
Memory used 277,760k (± 0.02%) 277,735k (± 0.02%) -25k (- 0.01%) 277,569k 277,821k
Parse Time 2.47s (± 0.66%) 2.46s (± 0.49%) -0.01s (- 0.28%) 2.44s 2.49s
Bind Time 0.72s (± 5.52%) 0.70s (± 3.59%) -0.02s (- 2.77%) 0.68s 0.80s
Check Time 16.44s (± 0.63%) 16.39s (± 0.63%) -0.05s (- 0.32%) 16.28s 16.78s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.63s (± 0.53%) 19.55s (± 0.49%) -0.08s (- 0.39%) 19.43s 19.91s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 40778 10
Baseline master 10

@typescript-bot typescript-bot mentioned this pull request Sep 25, 2020
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.

@amcasey
Copy link
Member

amcasey commented Sep 25, 2020

Looks good in the project that inspired the bug.

@sandersn sandersn assigned ahejlsberg and unassigned weswigham Oct 9, 2020
@ahejlsberg
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 10, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 10, 2020 •
edited
Loading

Hey @ahejlsberg, 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/87440/artifacts?artifactName=tgz&fileId=10CFBA909154FC3A3E00AF7B6BF5732CA0642D32E0BA4C34B51C92475AC34DC802&fileName=/typescript-4.1.0-insiders.20201010.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

@ahejlsberg ahejlsberg mentioned this pull request Oct 12, 2020
Closed
@weswigham
Copy link
Member Author

Do we want this in 4.1 or in 4.2?

@ahejlsberg
Copy link
Member

I vote 4.1.

@amcasey
Copy link
Member

amcasey commented Oct 12, 2020

FWIW, the design notes suggested we settled on 4.2.

@weswigham
Copy link
Member Author

weswigham commented Oct 12, 2020 •
edited
Loading

Nobody seemed to object to moving this to 4.1 in today's standup, so I'm assuming I should do so.

@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 12, 2020
@weswigham weswigham merged commit 4bf8ac2 into microsoft:master Oct 12, 2020
@weswigham weswigham deleted the spread-compact-literals branch October 12, 2020 22:40
@avaly avaly mentioned this pull request Nov 24, 2020
Closed
@github-actions github-actions bot mentioned this pull request Jul 14, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahejlsberg ahejlsberg ahejlsberg approved these changes

@amcasey amcasey Awaiting requested review from amcasey

Assignees

@ahejlsberg ahejlsberg

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Repeated conditional spreading of object literals results in OOM
4 participants
@weswigham @typescript-bot @amcasey @ahejlsberg