Skip to content

Fix control flow analysis in try-catch-finally #34880

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

Merged
merged 3 commits into from
Nov 5, 2019
Merged

Fix control flow analysis in try-catch-finally #34880

merged 3 commits into from
Nov 5, 2019

Conversation

ahejlsberg
Copy link
Member

This PR revises our logic for building CFA graphs for try-catch-finally statements. Two changes are implemented by this PR:

  • Previously we'd include every CFA node in the try block as a possible antecedent in the exception case. Now we only include CFA nodes representing assignments and array mutations.
  • Previously we didn't consider the possible effects in the finally block of exceptions in the catch block. We now do.

Fixes #34797.

@ahejlsberg
Copy link
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 2, 2019

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 2, 2019

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f2b3e06. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 2, 2019

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..34880

Metric master 34880 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 355,152k (± 0.01%) 356,855k (± 0.02%) +1,703k (+ 0.48%) 356,657k 356,985k
Parse Time 1.62s (± 0.65%) 1.63s (± 0.52%) +0.01s (+ 0.49%) 1.62s 1.65s
Bind Time 0.87s (± 1.16%) 0.87s (± 0.74%) +0.01s (+ 0.69%) 0.86s 0.88s
Check Time 4.53s (± 0.59%) 4.56s (± 0.41%) +0.03s (+ 0.64%) 4.53s 4.61s
Emit Time 5.28s (± 0.72%) 5.31s (± 0.71%) +0.02s (+ 0.47%) 5.23s 5.39s
Total Time 12.30s (± 0.51%) 12.37s (± 0.35%) +0.07s (+ 0.56%) 12.25s 12.50s
Monaco - node (v10.16.3, x64)
Memory used 366,012k (± 0.02%) 365,892k (± 0.01%) -121k (- 0.03%) 365,806k 365,971k
Parse Time 1.27s (± 0.76%) 1.26s (± 0.63%) -0.01s (- 0.71%) 1.25s 1.28s
Bind Time 0.76s (± 0.39%) 0.75s (± 0.59%) -0.01s (- 0.79%) 0.74s 0.76s
Check Time 4.65s (± 0.69%) 4.65s (± 0.80%) -0.00s (- 0.02%) 4.56s 4.71s
Emit Time 2.96s (± 0.74%) 2.95s (± 0.54%) -0.01s (- 0.34%) 2.93s 3.00s
Total Time 9.64s (± 0.42%) 9.62s (± 0.48%) -0.03s (- 0.27%) 9.54s 9.74s
TFS - node (v10.16.3, x64)
Memory used 321,701k (± 0.02%) 321,638k (± 0.02%) -63k (- 0.02%) 321,470k 321,785k
Parse Time 0.96s (± 0.71%) 0.96s (± 0.79%) 0.00s ( 0.00%) 0.95s 0.98s
Bind Time 0.72s (± 1.31%) 0.73s (± 1.03%) +0.00s (+ 0.41%) 0.70s 0.74s
Check Time 4.12s (± 0.40%) 4.11s (± 0.68%) -0.02s (- 0.41%) 4.06s 4.20s
Emit Time 3.06s (± 0.63%) 3.07s (± 0.76%) +0.00s (+ 0.13%) 3.02s 3.12s
Total Time 8.88s (± 0.33%) 8.86s (± 0.51%) -0.01s (- 0.15%) 8.79s 8.98s
Angular - node (v12.1.0, x64)
Memory used 330,667k (± 0.04%) 332,349k (± 0.02%) +1,682k (+ 0.51%) 332,171k 332,468k
Parse Time 1.56s (± 0.48%) 1.57s (± 0.42%) +0.01s (+ 0.58%) 1.56s 1.59s
Bind Time 0.85s (± 0.79%) 0.85s (± 0.70%) +0.01s (+ 0.83%) 0.84s 0.87s
Check Time 4.47s (± 0.66%) 4.44s (± 0.41%) -0.02s (- 0.54%) 4.41s 4.49s
Emit Time 5.48s (± 0.97%) 5.48s (± 0.95%) +0.00s (+ 0.05%) 5.42s 5.61s
Total Time 12.36s (± 0.62%) 12.35s (± 0.50%) -0.01s (- 0.06%) 12.26s 12.51s
Monaco - node (v12.1.0, x64)
Memory used 345,751k (± 0.01%) 345,711k (± 0.01%) -40k (- 0.01%) 345,629k 345,824k
Parse Time 1.23s (± 0.75%) 1.23s (± 0.69%) -0.00s (- 0.24%) 1.21s 1.25s
Bind Time 0.72s (± 0.51%) 0.73s (± 0.61%) +0.00s (+ 0.41%) 0.72s 0.74s
Check Time 4.50s (± 0.37%) 4.49s (± 0.47%) -0.01s (- 0.16%) 4.43s 4.54s
Emit Time 3.01s (± 0.57%) 3.03s (± 0.96%) +0.01s (+ 0.46%) 2.95s 3.08s
Total Time 9.47s (± 0.29%) 9.47s (± 0.40%) +0.00s (+ 0.02%) 9.39s 9.56s
TFS - node (v12.1.0, x64)
Memory used 304,008k (± 0.02%) 304,032k (± 0.02%) +24k (+ 0.01%) 303,870k 304,185k
Parse Time 0.95s (± 0.61%) 0.96s (± 0.95%) +0.01s (+ 0.95%) 0.94s 0.98s
Bind Time 0.69s (± 0.94%) 0.68s (± 0.70%) -0.01s (- 0.73%) 0.67s 0.69s
Check Time 4.05s (± 0.62%) 4.04s (± 0.61%) -0.01s (- 0.20%) 4.00s 4.12s
Emit Time 3.10s (± 1.05%) 3.09s (± 0.81%) -0.01s (- 0.23%) 3.05s 3.17s
Total Time 8.79s (± 0.53%) 8.78s (± 0.47%) -0.01s (- 0.08%) 8.70s 8.88s
Angular - node (v8.9.0, x64)
Memory used 349,951k (± 0.02%) 351,588k (± 0.02%) +1,637k (+ 0.47%) 351,363k 351,718k
Parse Time 2.11s (± 0.48%) 2.12s (± 0.47%) +0.01s (+ 0.66%) 2.10s 2.15s
Bind Time 0.91s (± 0.80%) 0.92s (± 0.88%) +0.01s (+ 1.32%) 0.90s 0.94s
Check Time 5.28s (± 0.36%) 5.31s (± 0.36%) +0.03s (+ 0.49%) 5.27s 5.35s
Emit Time 6.26s (± 0.66%) 6.28s (± 0.95%) +0.03s (+ 0.43%) 6.09s 6.37s
Total Time 14.55s (± 0.30%) 14.63s (± 0.42%) +0.08s (+ 0.56%) 14.42s 14.74s
Monaco - node (v8.9.0, x64)
Memory used 363,805k (± 0.01%) 363,781k (± 0.02%) -24k (- 0.01%) 363,600k 363,917k
Parse Time 1.57s (± 0.61%) 1.57s (± 0.67%) +0.00s (+ 0.26%) 1.56s 1.61s
Bind Time 0.93s (± 0.83%) 0.93s (± 0.75%) 0.00s ( 0.00%) 0.91s 0.94s
Check Time 5.56s (± 0.35%) 5.56s (± 0.68%) -0.00s (- 0.07%) 5.47s 5.66s
Emit Time 3.05s (± 0.69%) 3.06s (± 0.70%) +0.01s (+ 0.39%) 3.02s 3.12s
Total Time 11.11s (± 0.39%) 11.12s (± 0.43%) +0.01s (+ 0.09%) 11.02s 11.25s
TFS - node (v8.9.0, x64)
Memory used 320,571k (± 0.01%) 320,525k (± 0.01%) -46k (- 0.01%) 320,432k 320,607k
Parse Time 1.28s (± 0.29%) 1.27s (± 1.05%) -0.00s (- 0.08%) 1.25s 1.31s
Bind Time 0.74s (± 0.54%) 0.74s (± 0.70%) 0.00s ( 0.00%) 0.73s 0.75s
Check Time 4.76s (± 0.44%) 4.73s (± 0.50%) -0.03s (- 0.57%) 4.66s 4.79s
Emit Time 3.27s (± 1.00%) 3.22s (± 0.76%) -0.05s (- 1.62%) 3.15s 3.26s
Total Time 10.05s (± 0.47%) 9.97s (± 0.34%) -0.08s (- 0.81%) 9.89s 10.03s
Angular - node (v8.9.0, x86)
Memory used 198,699k (± 0.03%) 199,554k (± 0.04%) +855k (+ 0.43%) 199,284k 199,670k
Parse Time 2.04s (± 1.00%) 2.06s (± 0.82%) +0.02s (+ 1.13%) 2.02s 2.09s
Bind Time 1.03s (± 1.58%) 1.03s (± 0.80%) -0.01s (- 0.77%) 1.01s 1.04s
Check Time 4.82s (± 0.63%) 4.81s (± 0.54%) -0.01s (- 0.15%) 4.77s 4.88s
Emit Time 6.07s (± 1.16%) 6.13s (± 1.30%) +0.06s (+ 1.02%) 6.04s 6.36s
Total Time 13.96s (± 0.62%) 14.03s (± 0.54%) +0.07s (+ 0.49%) 13.91s 14.21s
Monaco - node (v8.9.0, x86)
Memory used 203,783k (± 0.02%) 203,749k (± 0.02%) -34k (- 0.02%) 203,663k 203,810k
Parse Time 1.62s (± 0.40%) 1.63s (± 1.05%) +0.01s (+ 0.43%) 1.60s 1.68s
Bind Time 0.75s (± 1.08%) 0.75s (± 0.63%) +0.00s (+ 0.40%) 0.74s 0.76s
Check Time 5.36s (± 0.77%) 5.43s (± 0.34%) +0.07s (+ 1.27%) 5.40s 5.48s
Emit Time 2.93s (± 1.25%) 2.89s (± 1.26%) -0.04s (- 1.47%) 2.83s 3.01s
Total Time 10.67s (± 0.39%) 10.70s (± 0.39%) +0.03s (+ 0.30%) 10.63s 10.80s
TFS - node (v8.9.0, x86)
Memory used 180,549k (± 0.02%) 180,551k (± 0.02%) +2k (+ 0.00%) 180,471k 180,633k
Parse Time 1.31s (± 0.83%) 1.32s (± 0.68%) +0.01s (+ 0.61%) 1.30s 1.33s
Bind Time 0.70s (± 0.71%) 0.70s (± 0.57%) +0.01s (+ 0.72%) 0.69s 0.71s
Check Time 4.51s (± 0.68%) 4.49s (± 0.63%) -0.01s (- 0.27%) 4.43s 4.55s
Emit Time 2.97s (± 1.03%) 2.99s (± 0.83%) +0.01s (+ 0.40%) 2.94s 3.06s
Total Time 9.48s (± 0.52%) 9.50s (± 0.64%) +0.02s (+ 0.16%) 9.37s 9.64s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory8 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)
Benchmark Name Iterations
Current 34880 10
Baseline master 10

@ahejlsberg
Copy link
Member Author

Tests all look good (DT failures are pre-existing conditions).

@ahejlsberg ahejlsberg merged commit 1c42c1a into master Nov 5, 2019
@ahejlsberg ahejlsberg deleted the fix34797 branch November 5, 2019 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throw in catch causes type narrowing in finally block
3 participants