Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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