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.

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

Fix crash checking spread element in loop #31098

Merged
merged 2 commits into from May 7, 2019

Conversation

andrewbranch
Copy link
Member

Fixes #30804

I don't really understand this, but was tipped off by #25228. I'm open to suggestions for a better fix for this class of crash.

@andrewbranch
Copy link
Member Author

@typescript-bot perf test this please

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 24, 2019

Heya @andrewbranch, I've started to run the perf test suite on this PR at 89497fc. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@sandersn
Copy link
Member

sandersn commented Apr 24, 2019

@weswigham didn't you have some idea to squash this class of bugs once and for all. I remember trying something a year or two ago that seemed to work but was scary in its own right.

Until then, switching away from checkExpressionCached is the right thing.

Edit: Can't find it. Maybe I didn't make a PR for it?

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..31098

Metric master 31098 Delta Best Worst
Angular - node (v6.5.0, x64)
Memory used 341,063k (± 0.43%) 341,421k (± 0.45%) +357k (+ 0.10%) 339,117k 343,514k
Parse Time 1.63s (± 0.61%) 1.59s (± 1.90%) -0.04s (- 2.52%) 1.54s 1.63s
Bind Time 1.54s (± 0.97%) 1.60s (± 2.81%) +0.06s (+ 4.10%) 1.54s 1.69s
Check Time 4.86s (± 0.49%) 4.84s (± 0.42%) -0.02s (- 0.41%) 4.80s 4.88s
Emit Time 6.37s (± 1.72%) 6.39s (± 2.13%) +0.02s (+ 0.31%) 6.16s 6.63s
Total Time 14.40s (± 0.82%) 14.42s (± 0.94%) +0.02s (+ 0.15%) 14.19s 14.70s
Monaco - node (v6.5.0, x64)
Memory used 364,356k (± 0.09%) 364,340k (± 0.09%) -16k (- 0.00%) 364,047k 365,593k
Parse Time 1.27s (± 0.57%) 1.27s (± 0.38%) -0.00s (- 0.08%) 1.26s 1.28s
Bind Time 1.52s (± 0.57%) 1.51s (± 0.50%) -0.01s (- 0.72%) 1.49s 1.53s
Check Time 5.68s (± 2.76%) 5.68s (± 1.13%) +0.01s (+ 0.11%) 5.45s 5.78s
Emit Time 3.59s (± 2.96%) 3.59s (± 2.29%) -0.01s (- 0.19%) 3.33s 3.71s
Total Time 12.05s (± 2.10%) 12.04s (± 0.98%) -0.01s (- 0.08%) 11.70s 12.19s
TFS - node (v6.5.0, x64)
Memory used 321,067k (± 0.00%) 321,065k (± 0.00%) -1k (- 0.00%) 321,049k 321,095k
Parse Time 0.98s (± 0.46%) 0.98s (± 0.37%) -0.00s (- 0.20%) 0.97s 0.98s
Bind Time 1.27s (± 0.97%) 1.28s (± 0.94%) +0.01s (+ 0.86%) 1.27s 1.32s
Check Time 4.36s (± 0.59%) 4.35s (± 0.32%) -0.02s (- 0.37%) 4.33s 4.39s
Emit Time 3.13s (± 0.90%) 3.13s (± 0.55%) -0.01s (- 0.19%) 3.09s 3.16s
Total Time 9.75s (± 0.32%) 9.73s (± 0.24%) -0.01s (- 0.12%) 9.67s 9.76s
Angular - node (v6.5.0, x86)
Memory used 191,016k (± 0.00%) 191,006k (± 0.01%) -10k (- 0.01%) 190,980k 191,026k
Parse Time 1.47s (± 0.91%) 1.45s (± 0.77%) -0.02s (- 1.09%) 1.43s 1.47s
Bind Time 1.50s (± 0.99%) 1.50s (± 0.44%) +0.01s (+ 0.40%) 1.48s 1.51s
Check Time 4.76s (± 1.18%) 4.74s (± 0.38%) -0.01s (- 0.29%) 4.71s 4.79s
Emit Time 6.23s (± 1.66%) 6.11s (± 1.79%) -0.12s (- 1.94%) 5.87s 6.38s
Total Time 13.95s (± 0.91%) 13.80s (± 0.84%) -0.14s (- 1.01%) 13.58s 14.07s
Monaco - node (v6.5.0, x86)
Memory used 204,356k (± 0.01%) 204,343k (± 0.01%) -13k (- 0.01%) 204,308k 204,376k
Parse Time 1.26s (± 0.56%) 1.27s (± 0.60%) +0.00s (+ 0.40%) 1.25s 1.28s
Bind Time 1.55s (± 0.68%) 1.55s (± 0.49%) -0.01s (- 0.45%) 1.53s 1.56s
Check Time 4.64s (± 1.59%) 4.69s (± 1.42%) +0.04s (+ 0.97%) 4.62s 4.90s
Emit Time 3.13s (± 4.48%) 3.16s (± 2.09%) +0.03s (+ 0.86%) 2.96s 3.24s
Total Time 10.59s (± 1.00%) 10.66s (± 0.31%) +0.07s (+ 0.68%) 10.57s 10.73s
TFS - node (v6.5.0, x86)
Memory used 180,330k (± 0.01%) 180,338k (± 0.01%) +8k (+ 0.00%) 180,295k 180,372k
Parse Time 0.99s (± 0.84%) 0.98s (± 0.59%) -0.01s (- 0.91%) 0.97s 0.99s
Bind Time 1.31s (± 0.84%) 1.30s (± 0.71%) -0.02s (- 1.14%) 1.28s 1.32s
Check Time 3.92s (± 0.38%) 3.93s (± 0.70%) +0.02s (+ 0.49%) 3.89s 4.02s
Emit Time 2.56s (± 0.62%) 2.58s (± 1.39%) +0.02s (+ 0.82%) 2.49s 2.67s
Total Time 8.78s (± 0.32%) 8.79s (± 0.58%) +0.01s (+ 0.15%) 8.66s 8.89s
Angular - node (v8.9.0, x64)
Memory used 330,708k (± 0.02%) 330,719k (± 0.01%) +11k (+ 0.00%) 330,632k 330,821k
Parse Time 1.80s (± 0.58%) 1.80s (± 0.45%) +0.00s (+ 0.06%) 1.78s 1.82s
Bind Time 1.36s (± 1.30%) 1.36s (± 0.55%) -0.00s (- 0.15%) 1.34s 1.38s
Check Time 4.64s (± 1.83%) 4.61s (± 1.29%) -0.03s (- 0.56%) 4.51s 4.73s
Emit Time 6.07s (± 2.63%) 5.97s (± 2.75%) -0.09s (- 1.57%) 5.64s 6.30s
Total Time 13.86s (± 0.85%) 13.75s (± 0.82%) -0.12s (- 0.84%) 13.45s 13.98s
Monaco - node (v8.9.0, x64)
Memory used 358,474k (± 0.02%) 358,439k (± 0.02%) -35k (- 0.01%) 358,319k 358,593k
Parse Time 1.45s (± 0.47%) 1.45s (± 0.47%) +0.00s (+ 0.00%) 1.43s 1.46s
Bind Time 1.52s (± 0.88%) 1.52s (± 0.63%) +0.01s (+ 0.40%) 1.50s 1.54s
Check Time 4.86s (± 1.71%) 4.81s (± 1.40%) -0.05s (- 0.95%) 4.74s 5.04s
Emit Time 3.01s (± 5.55%) 3.20s (± 4.19%) +0.19s (+ 6.38%) 2.83s 3.31s
Total Time 10.83s (± 0.87%) 10.98s (± 0.77%) +0.15s (+ 1.39%) 10.70s 11.07s
TFS - node (v8.9.0, x64)
Memory used 313,833k (± 0.02%) 313,824k (± 0.01%) -9k (- 0.00%) 313,731k 313,882k
Parse Time 1.15s (± 0.66%) 1.14s (± 0.43%) -0.00s (- 0.17%) 1.14s 1.16s
Bind Time 1.24s (± 1.77%) 1.23s (± 0.94%) -0.01s (- 0.65%) 1.21s 1.26s
Check Time 4.21s (± 1.02%) 4.21s (± 0.72%) +0.01s (+ 0.14%) 4.17s 4.31s
Emit Time 3.12s (± 0.68%) 3.15s (± 0.84%) +0.03s (+ 0.83%) 3.09s 3.21s
Total Time 9.71s (± 0.65%) 9.73s (± 0.46%) +0.03s (+ 0.26%) 9.68s 9.85s
Angular - node (v8.9.0, x86)
Memory used 187,126k (± 0.02%) 187,152k (± 0.02%) +26k (+ 0.01%) 187,046k 187,216k
Parse Time 1.74s (± 0.48%) 1.75s (± 0.57%) +0.01s (+ 0.46%) 1.73s 1.78s
Bind Time 1.52s (± 0.44%) 1.53s (± 0.71%) +0.00s (+ 0.33%) 1.50s 1.56s
Check Time 4.28s (± 0.64%) 4.27s (± 0.39%) -0.01s (- 0.19%) 4.24s 4.31s
Emit Time 5.69s (± 0.80%) 5.60s (± 1.27%) -0.09s (- 1.63%) 5.48s 5.75s
Total Time 13.24s (± 0.36%) 13.15s (± 0.65%) -0.09s (- 0.64%) 13.00s 13.35s
Monaco - node (v8.9.0, x86)
Memory used 199,834k (± 0.02%) 199,827k (± 0.02%) -7k (- 0.00%) 199,741k 199,934k
Parse Time 1.50s (± 0.53%) 1.51s (± 0.50%) +0.00s (+ 0.33%) 1.50s 1.53s
Bind Time 1.39s (± 0.50%) 1.39s (± 0.44%) -0.00s (- 0.14%) 1.37s 1.40s
Check Time 4.62s (± 0.47%) 4.61s (± 0.54%) -0.02s (- 0.35%) 4.56s 4.67s
Emit Time 3.09s (± 1.21%) 3.11s (± 1.28%) +0.01s (+ 0.45%) 3.03s 3.23s
Total Time 10.61s (± 0.47%) 10.61s (± 0.50%) -0.00s (- 0.04%) 10.52s 10.76s
TFS - node (v8.9.0, x86)
Memory used 175,898k (± 0.02%) 175,869k (± 0.01%) -29k (- 0.02%) 175,813k 175,919k
Parse Time 1.20s (± 1.02%) 1.20s (± 0.76%) 0.00s ( 0.00%) 1.18s 1.22s
Bind Time 1.24s (± 0.71%) 1.23s (± 0.55%) -0.00s (- 0.24%) 1.22s 1.25s
Check Time 4.01s (± 0.99%) 4.02s (± 1.07%) +0.01s (+ 0.15%) 3.97s 4.15s
Emit Time 2.74s (± 1.06%) 2.78s (± 0.87%) +0.04s (+ 1.38%) 2.73s 2.83s
Total Time 9.19s (± 0.76%) 9.23s (± 0.60%) +0.04s (+ 0.41%) 9.14s 9.37s
Angular - node (v9.0.0, x64)
Memory used 330,409k (± 0.02%) 330,424k (± 0.02%) +14k (+ 0.00%) 330,332k 330,577k
Parse Time 1.64s (± 0.38%) 1.63s (± 0.29%) -0.01s (- 0.37%) 1.62s 1.64s
Bind Time 1.33s (± 0.56%) 1.33s (± 0.37%) +0.00s (+ 0.15%) 1.32s 1.34s
Check Time 4.32s (± 0.50%) 4.34s (± 0.50%) +0.01s (+ 0.32%) 4.28s 4.37s
Emit Time 5.72s (± 1.06%) 5.74s (± 2.10%) +0.03s (+ 0.44%) 5.59s 6.11s
Total Time 13.01s (± 0.52%) 13.05s (± 0.96%) +0.03s (+ 0.26%) 12.87s 13.39s
Monaco - node (v9.0.0, x64)
Memory used 358,185k (± 0.01%) 358,189k (± 0.02%) +5k (+ 0.00%) 358,026k 358,267k
Parse Time 1.29s (± 0.36%) 1.29s (± 0.60%) -0.00s (- 0.08%) 1.28s 1.31s
Bind Time 1.51s (± 0.65%) 1.52s (± 0.98%) +0.01s (+ 0.53%) 1.49s 1.55s
Check Time 4.69s (± 0.54%) 4.67s (± 0.28%) -0.03s (- 0.58%) 4.63s 4.70s
Emit Time 3.22s (± 0.59%) 3.22s (± 0.43%) +0.00s (+ 0.12%) 3.19s 3.26s
Total Time 10.71s (± 0.41%) 10.69s (± 0.17%) -0.01s (- 0.14%) 10.65s 10.73s
TFS - node (v9.0.0, x64)
Memory used 313,766k (± 0.01%) 313,801k (± 0.02%) +35k (+ 0.01%) 313,665k 313,934k
Parse Time 1.01s (± 0.59%) 1.02s (± 0.61%) +0.00s (+ 0.39%) 1.01s 1.03s
Bind Time 1.20s (± 0.74%) 1.21s (± 1.45%) +0.00s (+ 0.33%) 1.18s 1.27s
Check Time 4.25s (± 1.65%) 4.21s (± 1.37%) -0.04s (- 0.99%) 4.13s 4.38s
Emit Time 2.98s (± 3.28%) 3.03s (± 2.14%) +0.05s (+ 1.68%) 2.89s 3.15s
Total Time 9.45s (± 0.36%) 9.46s (± 0.36%) +0.01s (+ 0.15%) 9.39s 9.54s
Angular - node (v9.0.0, x86)
Memory used 187,335k (± 0.03%) 187,316k (± 0.03%) -19k (- 0.01%) 187,206k 187,482k
Parse Time 1.56s (± 0.68%) 1.55s (± 0.34%) -0.00s (- 0.26%) 1.54s 1.56s
Bind Time 1.51s (± 0.72%) 1.51s (± 0.27%) -0.00s (- 0.20%) 1.50s 1.52s
Check Time 4.04s (± 0.99%) 4.04s (± 1.03%) +0.00s (+ 0.02%) 3.96s 4.16s
Emit Time 5.36s (± 0.64%) 5.31s (± 0.55%) -0.05s (- 0.88%) 5.24s 5.37s
Total Time 12.46s (± 0.58%) 12.41s (± 0.50%) -0.05s (- 0.39%) 12.30s 12.58s
Monaco - node (v9.0.0, x86)
Memory used 199,885k (± 0.01%) 199,946k (± 0.02%) +61k (+ 0.03%) 199,858k 200,024k
Parse Time 1.32s (± 0.44%) 1.33s (± 1.03%) +0.00s (+ 0.30%) 1.30s 1.37s
Bind Time 1.36s (± 0.56%) 1.36s (± 1.00%) -0.00s (- 0.15%) 1.34s 1.39s
Check Time 4.46s (± 0.34%) 4.50s (± 0.44%) +0.04s (+ 0.96%) 4.46s 4.54s
Emit Time 3.02s (± 0.65%) 3.01s (± 0.42%) -0.01s (- 0.33%) 2.98s 3.03s
Total Time 10.16s (± 0.30%) 10.20s (± 0.36%) +0.04s (+ 0.39%) 10.14s 10.30s
TFS - node (v9.0.0, x86)
Memory used 176,010k (± 0.02%) 175,995k (± 0.02%) -16k (- 0.01%) 175,906k 176,057k
Parse Time 1.03s (± 0.66%) 1.04s (± 0.90%) +0.01s (+ 0.87%) 1.02s 1.07s
Bind Time 1.22s (± 1.13%) 1.22s (± 1.20%) 0.00s ( 0.00%) 1.20s 1.27s
Check Time 3.90s (± 0.50%) 3.91s (± 0.61%) +0.01s (+ 0.26%) 3.87s 3.97s
Emit Time 2.70s (± 0.69%) 2.72s (± 1.37%) +0.03s (+ 0.96%) 2.67s 2.86s
Total Time 8.85s (± 0.43%) 8.89s (± 0.63%) +0.05s (+ 0.51%) 8.80s 9.07s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-142-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v6.5.0, x64)
  • node (v6.5.0, x86)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
  • node (v9.0.0, x64)
  • node (v9.0.0, x86)
Scenarios
  • Angular - node (v6.5.0, x64)
  • Angular - node (v6.5.0, x86)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Angular - node (v9.0.0, x64)
  • Angular - node (v9.0.0, x86)
  • Monaco - node (v6.5.0, x64)
  • Monaco - node (v6.5.0, x86)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • Monaco - node (v9.0.0, x64)
  • Monaco - node (v9.0.0, x86)
  • TFS - node (v6.5.0, x64)
  • TFS - node (v6.5.0, x86)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • TFS - node (v9.0.0, x64)
  • TFS - node (v9.0.0, x86)
Benchmark Name Iterations
Current 31098 10
Baseline master 10

@andrewbranch
Copy link
Member Author

@sandersn @weswigham is this good to go or did you have any suggestions?

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I'm good with the change but get @weswigham to sign off too, since he's looked at this more recently than I have.

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.

didn't you have some idea to squash this class of bugs once and for all. I remember trying something a year or two ago that seemed to work but was scary in its own right.

A structured cache for resolved types, this way each time we enter into a new context we push a new "layer" and if it's finalized (accepted as a correct and final result) we merge the cache layer down. It's not something we currently want because of the implied complexity, so these kinds of ad hoc fixes are all we have right now. T.T

@andrewbranch andrewbranch merged commit 8c07b40 into microsoft:master May 7, 2019
@andrewbranch andrewbranch deleted the bug/30804 branch May 7, 2019 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants