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

Cache property name widening contexts #40732

Closed
wants to merge 1 commit into from
Closed

Cache property name widening contexts #40732

wants to merge 1 commit into from

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Sep 24, 2020

When creating a new WideningContext for a given property, check whether
there's an existing WideningContext with the same parent and
propertyName. Otherwise, we won't adequately leverage the cache of
computed siblings stored on the WideningContext.

On my box, check time drops from 10 seconds to 0.3 seconds for this example.

@amcasey
Cache property name widening contexts
647421f 
When creating a new `WideningContext` for a given property, check whether
there's an existing `WideningContext` with the same `parent` and
`propertyName`.  Otherwise, we won't adequately leverage the cache of
computed `siblings` stored on the `WideningContext`.
@amcasey amcasey requested a review from ahejlsberg September 24, 2020 00:31
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 24, 2020
@amcasey
Copy link
Member Author

amcasey commented Sep 24, 2020

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 24, 2020 •
edited
Loading

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

Update: The results are in!

@amcasey
Copy link
Member Author

amcasey commented Sep 24, 2020

Here's a cleaned up and annotated repro.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..40732

Metric master 40732 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 349,562k (± 0.02%) 349,600k (± 0.02%) +38k (+ 0.01%) 349,512k 349,747k
Parse Time 2.01s (± 0.64%) 2.00s (± 0.82%) -0.01s (- 0.50%) 1.97s 2.03s
Bind Time 0.83s (± 1.00%) 0.83s (± 0.84%) +0.01s (+ 0.61%) 0.82s 0.85s
Check Time 4.90s (± 0.72%) 4.93s (± 0.82%) +0.03s (+ 0.63%) 4.85s 5.04s
Emit Time 5.22s (± 1.07%) 5.21s (± 0.57%) -0.01s (- 0.19%) 5.15s 5.27s
Total Time 12.95s (± 0.63%) 12.97s (± 0.56%) +0.02s (+ 0.12%) 12.84s 13.17s
Monaco - node (v10.16.3, x64)
Memory used 354,401k (± 0.03%) 354,335k (± 0.02%) -66k (- 0.02%) 354,145k 354,478k
Parse Time 1.57s (± 0.80%) 1.56s (± 0.44%) -0.02s (- 1.14%) 1.54s 1.57s
Bind Time 0.72s (± 0.93%) 0.71s (± 0.42%) -0.00s (- 0.70%) 0.71s 0.72s
Check Time 5.07s (± 0.42%) 5.05s (± 0.38%) -0.02s (- 0.32%) 5.02s 5.12s
Emit Time 2.77s (± 0.81%) 2.75s (± 0.71%) -0.01s (- 0.47%) 2.71s 2.81s
Total Time 10.13s (± 0.44%) 10.07s (± 0.26%) -0.05s (- 0.53%) 10.02s 10.15s
TFS - node (v10.16.3, x64)
Memory used 307,561k (± 0.02%) 307,564k (± 0.03%) +3k (+ 0.00%) 307,425k 307,895k
Parse Time 1.22s (± 0.85%) 1.22s (± 0.63%) -0.01s (- 0.57%) 1.20s 1.23s
Bind Time 0.67s (± 0.99%) 0.67s (± 1.30%) -0.00s (- 0.74%) 0.64s 0.68s
Check Time 4.52s (± 0.52%) 4.53s (± 0.53%) +0.01s (+ 0.20%) 4.48s 4.57s
Emit Time 2.90s (± 0.85%) 2.89s (± 0.80%) -0.01s (- 0.45%) 2.82s 2.92s
Total Time 9.31s (± 0.39%) 9.30s (± 0.39%) -0.01s (- 0.14%) 9.20s 9.36s
material-ui - node (v10.16.3, x64)
Memory used 488,979k (± 0.02%) 488,998k (± 0.01%) +20k (+ 0.00%) 488,873k 489,118k
Parse Time 1.98s (± 0.47%) 1.98s (± 0.33%) +0.00s (+ 0.10%) 1.97s 1.99s
Bind Time 0.65s (± 0.76%) 0.65s (± 0.95%) +0.00s (+ 0.46%) 0.63s 0.66s
Check Time 13.43s (± 0.58%) 13.42s (± 0.41%) -0.01s (- 0.07%) 13.35s 13.55s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.06s (± 0.53%) 16.05s (± 0.38%) -0.00s (- 0.02%) 15.96s 16.20s
Angular - node (v12.1.0, x64)
Memory used 326,813k (± 0.02%) 326,830k (± 0.02%) +17k (+ 0.01%) 326,700k 326,970k
Parse Time 2.00s (± 0.47%) 2.00s (± 0.41%) -0.00s (- 0.15%) 1.98s 2.02s
Bind Time 0.81s (± 0.64%) 0.81s (± 0.64%) +0.00s (+ 0.25%) 0.80s 0.82s
Check Time 4.78s (± 0.46%) 4.79s (± 0.61%) +0.01s (+ 0.23%) 4.76s 4.90s
Emit Time 5.33s (± 0.61%) 5.35s (± 0.41%) +0.02s (+ 0.38%) 5.30s 5.39s
Total Time 12.92s (± 0.31%) 12.95s (± 0.36%) +0.03s (+ 0.23%) 12.85s 13.06s
Monaco - node (v12.1.0, x64)
Memory used 336,487k (± 0.02%) 336,522k (± 0.02%) +35k (+ 0.01%) 336,389k 336,718k
Parse Time 1.55s (± 0.82%) 1.53s (± 0.82%) -0.01s (- 0.90%) 1.51s 1.57s
Bind Time 0.70s (± 1.09%) 0.69s (± 1.67%) -0.00s (- 0.00%) 0.68s 0.74s
Check Time 4.88s (± 0.47%) 4.84s (± 0.44%) -0.04s (- 0.76%) 4.80s 4.90s
Emit Time 2.81s (± 0.65%) 2.80s (± 0.62%) -0.01s (- 0.25%) 2.75s 2.83s
Total Time 9.93s (± 0.40%) 9.87s (± 0.30%) -0.05s (- 0.55%) 9.79s 9.95s
TFS - node (v12.1.0, x64)
Memory used 291,891k (± 0.02%) 291,855k (± 0.01%) -36k (- 0.01%) 291,774k 291,954k
Parse Time 1.24s (± 0.70%) 1.23s (± 0.76%) -0.00s (- 0.40%) 1.21s 1.25s
Bind Time 0.64s (± 0.62%) 0.64s (± 1.13%) +0.00s (+ 0.16%) 0.63s 0.66s
Check Time 4.45s (± 0.38%) 4.43s (± 0.48%) -0.02s (- 0.52%) 4.39s 4.49s
Emit Time 2.91s (± 0.85%) 2.94s (± 0.70%) +0.02s (+ 0.82%) 2.88s 2.97s
Total Time 9.24s (± 0.38%) 9.24s (± 0.30%) -0.01s (- 0.06%) 9.15s 9.29s
material-ui - node (v12.1.0, x64)
Memory used 467,090k (± 0.01%) 466,946k (± 0.05%) -143k (- 0.03%) 465,988k 467,274k
Parse Time 2.00s (± 0.57%) 2.00s (± 0.63%) -0.00s (- 0.15%) 1.97s 2.03s
Bind Time 0.64s (± 0.77%) 0.63s (± 0.57%) -0.00s (- 0.31%) 0.63s 0.64s
Check Time 12.02s (± 0.69%) 12.06s (± 1.34%) +0.05s (+ 0.40%) 11.81s 12.66s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.66s (± 0.65%) 14.70s (± 1.09%) +0.04s (+ 0.29%) 14.45s 15.28s
Angular - node (v8.9.0, x64)
Memory used 346,294k (± 0.01%) 346,277k (± 0.02%) -16k (- 0.00%) 346,068k 346,423k
Parse Time 2.56s (± 0.43%) 2.54s (± 0.30%) -0.01s (- 0.51%) 2.53s 2.56s
Bind Time 0.87s (± 0.69%) 0.86s (± 0.64%) -0.00s (- 0.46%) 0.85s 0.88s
Check Time 5.53s (± 0.64%) 5.54s (± 0.35%) +0.00s (+ 0.04%) 5.49s 5.57s
Emit Time 6.07s (± 1.10%) 6.10s (± 2.14%) +0.03s (+ 0.54%) 5.87s 6.39s
Total Time 15.03s (± 0.51%) 15.04s (± 0.89%) +0.01s (+ 0.08%) 14.79s 15.33s
Monaco - node (v8.9.0, x64)
Memory used 355,599k (± 0.02%) 355,645k (± 0.02%) +45k (+ 0.01%) 355,478k 355,796k
Parse Time 1.88s (± 0.40%) 1.89s (± 1.25%) +0.01s (+ 0.53%) 1.86s 1.98s
Bind Time 0.89s (± 0.91%) 0.89s (± 0.65%) -0.00s (- 0.34%) 0.88s 0.90s
Check Time 5.61s (± 0.65%) 5.57s (± 0.42%) -0.03s (- 0.59%) 5.53s 5.64s
Emit Time 3.28s (± 1.50%) 3.27s (± 1.50%) -0.01s (- 0.21%) 3.20s 3.38s
Total Time 11.66s (± 0.40%) 11.63s (± 0.56%) -0.03s (- 0.29%) 11.50s 11.78s
TFS - node (v8.9.0, x64)
Memory used 309,269k (± 0.02%) 309,285k (± 0.01%) +15k (+ 0.00%) 309,207k 309,379k
Parse Time 1.56s (± 0.58%) 1.55s (± 0.31%) -0.00s (- 0.13%) 1.54s 1.56s
Bind Time 0.68s (± 0.49%) 0.68s (± 0.73%) -0.00s (- 0.59%) 0.67s 0.69s
Check Time 5.32s (± 0.31%) 5.30s (± 0.53%) -0.02s (- 0.38%) 5.23s 5.37s
Emit Time 2.95s (± 0.43%) 2.94s (± 0.75%) -0.01s (- 0.44%) 2.85s 2.96s
Total Time 10.50s (± 0.19%) 10.46s (± 0.39%) -0.04s (- 0.37%) 10.35s 10.55s
material-ui - node (v8.9.0, x64)
Memory used 493,307k (± 0.01%) 493,368k (± 0.01%) +61k (+ 0.01%) 493,259k 493,426k
Parse Time 2.40s (± 0.53%) 2.41s (± 0.25%) +0.00s (+ 0.12%) 2.39s 2.42s
Bind Time 0.82s (± 1.09%) 0.81s (± 0.99%) -0.00s (- 0.49%) 0.79s 0.83s
Check Time 17.98s (± 0.80%) 17.85s (± 0.91%) -0.13s (- 0.75%) 17.49s 18.15s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.20s (± 0.70%) 21.07s (± 0.76%) -0.13s (- 0.63%) 20.73s 21.38s
Angular - node (v8.9.0, x86)
Memory used 198,577k (± 0.02%) 198,641k (± 0.03%) +65k (+ 0.03%) 198,495k 198,722k
Parse Time 2.47s (± 0.57%) 2.48s (± 0.45%) +0.01s (+ 0.45%) 2.45s 2.50s
Bind Time 1.01s (± 0.84%) 1.00s (± 0.74%) -0.01s (- 0.59%) 0.98s 1.01s
Check Time 4.99s (± 0.51%) 4.98s (± 0.41%) -0.01s (- 0.20%) 4.93s 5.03s
Emit Time 5.89s (± 1.08%) 5.92s (± 1.24%) +0.02s (+ 0.42%) 5.77s 6.05s
Total Time 14.36s (± 0.56%) 14.39s (± 0.51%) +0.02s (+ 0.15%) 14.19s 14.52s
Monaco - node (v8.9.0, x86)
Memory used 201,404k (± 0.02%) 201,414k (± 0.02%) +10k (+ 0.00%) 201,305k 201,482k
Parse Time 1.93s (± 0.84%) 1.92s (± 0.68%) -0.01s (- 0.57%) 1.90s 1.95s
Bind Time 0.71s (± 0.63%) 0.71s (± 0.99%) +0.00s (+ 0.14%) 0.70s 0.73s
Check Time 5.46s (± 0.86%) 5.49s (± 1.10%) +0.02s (+ 0.46%) 5.41s 5.64s
Emit Time 3.01s (± 2.22%) 3.00s (± 4.00%) -0.01s (- 0.30%) 2.67s 3.16s
Total Time 11.11s (± 0.36%) 11.12s (± 0.68%) +0.01s (+ 0.04%) 10.92s 11.23s
TFS - node (v8.9.0, x86)
Memory used 176,799k (± 0.03%) 176,800k (± 0.02%) +1k (+ 0.00%) 176,753k 176,950k
Parse Time 1.59s (± 0.84%) 1.59s (± 0.73%) -0.01s (- 0.50%) 1.57s 1.63s
Bind Time 0.64s (± 0.81%) 0.64s (± 0.53%) +0.00s (+ 0.31%) 0.64s 0.65s
Check Time 4.81s (± 0.66%) 4.80s (± 0.66%) -0.01s (- 0.25%) 4.76s 4.90s
Emit Time 2.82s (± 1.16%) 2.80s (± 1.00%) -0.02s (- 0.71%) 2.73s 2.87s
Total Time 9.87s (± 0.59%) 9.83s (± 0.36%) -0.04s (- 0.42%) 9.75s 9.92s
material-ui - node (v8.9.0, x86)
Memory used 277,783k (± 0.01%) 277,765k (± 0.02%) -19k (- 0.01%) 277,610k 277,879k
Parse Time 2.47s (± 0.79%) 2.47s (± 0.82%) +0.01s (+ 0.28%) 2.43s 2.52s
Bind Time 0.72s (± 6.49%) 0.71s (± 4.27%) -0.01s (- 1.67%) 0.67s 0.82s
Check Time 16.38s (± 0.57%) 16.45s (± 0.75%) +0.07s (+ 0.44%) 16.06s 16.63s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.56s (± 0.49%) 19.63s (± 0.52%) +0.07s (+ 0.35%) 19.37s 19.85s
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 40732 10
Baseline master 10

@amcasey amcasey marked this pull request as ready for review September 24, 2020 17:50
@sandersn
Copy link
Member

sandersn commented Oct 9, 2020 •
edited
Loading

@amcasey seems like this is ready to merge? Or does it need to wait for 4.2?

@amcasey
Copy link
Member Author

amcasey commented Oct 9, 2020

@sandersn I was saving it for 4.2, but only because there's no rush.

@ahejlsberg
Copy link
Member

With #40778 the motivating example no longer creates a large union type, so I wonder if this PR is still worth doing?

@amcasey
Copy link
Member Author

amcasey commented Oct 12, 2020

@ahejlsberg I'm a little concerned that it'll bite us again in the future if we leave it quadratic, but I agree that there's no urgency. Your call.

@ahejlsberg
Copy link
Member

@amcasey The logic only kicks in for fresh object literal types and once #40778 goes in I'm not sure there's anything one would commonly (or even uncommonly) write that generates large unions of fresh object literals. So I vote we hold off.

@amcasey amcasey closed this Oct 12, 2020
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

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

@rbuckton rbuckton Awaiting requested review from rbuckton

Assignees
No one assigned
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amcasey @typescript-bot @sandersn @ahejlsberg @weswigham