Skip to content

Simplify constraint depth limiter logic #41972

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
Merged
merged 4 commits into from
Dec 16, 2020
Merged

Simplify constraint depth limiter logic #41972

merged 4 commits into from
Dec 16, 2020

Conversation

ahejlsberg
Copy link
Member

With this PR we always explore at least 10 levels of nested constraints. Thereafter, we continue to explore up to 50 levels of nested constraints provided there are no "deeply nested" types on the stack (i.e. no types for which five instantiations have been recorded on the stack). If we reach 50 levels of nesting, we are presumably exploring a repeating pattern with a long cycle that hasn't yet triggered the deeply nested limiter. We used to issue an error upon reaching 50 levels of nesting, but since the introduction of the depth limiter in #40971 we have no tests cases that get there, so I have removed that logic. Thus, infinite constraints are no longer considered an error, but circular constraints continue to be.

Fixes #41931.

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Dec 15, 2020
weswigham
weswigham approved these changes Dec 15, 2020
View reviewed changes
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.

We should still run this over dt and the like to see how well it works in the wild.

@ahejlsberg
Copy link
Member Author

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 15, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 15, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 15, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 15, 2020 •
edited
Loading

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

Update: The results are in!

@typescript-bot typescript-bot mentioned this pull request Dec 15, 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.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - master..41972

Metric master 41972 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 345,057k (± 0.02%) 345,035k (± 0.04%) -22k (- 0.01%) 344,769k 345,329k
Parse Time 1.99s (± 0.47%) 1.98s (± 0.59%) -0.01s (- 0.30%) 1.96s 2.01s
Bind Time 0.83s (± 0.63%) 0.83s (± 0.87%) +0.00s (+ 0.12%) 0.81s 0.84s
Check Time 5.00s (± 0.40%) 4.99s (± 0.38%) -0.01s (- 0.18%) 4.96s 5.05s
Emit Time 5.35s (± 1.02%) 5.39s (± 0.95%) +0.04s (+ 0.82%) 5.32s 5.52s
Total Time 13.16s (± 0.53%) 13.19s (± 0.53%) +0.03s (+ 0.23%) 13.07s 13.37s
Compiler-Unions - node (v10.16.3, x64)
Memory used 205,426k (± 0.05%) 205,417k (± 0.03%) -9k (- 0.00%) 205,278k 205,563k
Parse Time 0.79s (± 0.70%) 0.79s (± 1.02%) -0.00s (- 0.51%) 0.78s 0.81s
Bind Time 0.49s (± 0.74%) 0.51s (± 0.94%) +0.01s (+ 2.63%) 0.50s 0.52s
Check Time 12.03s (± 0.70%) 12.11s (± 0.98%) +0.08s (+ 0.67%) 11.97s 12.46s
Emit Time 2.35s (± 1.52%) 2.34s (± 0.99%) -0.01s (- 0.42%) 2.28s 2.39s
Total Time 15.67s (± 0.70%) 15.75s (± 0.78%) +0.08s (+ 0.52%) 15.58s 16.07s
Monaco - node (v10.16.3, x64)
Memory used 354,831k (± 0.03%) 354,944k (± 0.02%) +113k (+ 0.03%) 354,771k 355,205k
Parse Time 1.60s (± 0.42%) 1.60s (± 0.44%) -0.00s (- 0.06%) 1.58s 1.61s
Bind Time 0.73s (± 0.69%) 0.73s (± 0.76%) +0.01s (+ 0.97%) 0.72s 0.74s
Check Time 5.13s (± 0.54%) 5.13s (± 0.62%) -0.00s (- 0.02%) 5.09s 5.23s
Emit Time 2.81s (± 0.73%) 2.80s (± 0.46%) -0.00s (- 0.07%) 2.78s 2.83s
Total Time 10.26s (± 0.36%) 10.27s (± 0.45%) +0.01s (+ 0.06%) 10.18s 10.39s
TFS - node (v10.16.3, x64)
Memory used 307,884k (± 0.02%) 307,886k (± 0.04%) +2k (+ 0.00%) 307,716k 308,326k
Parse Time 1.23s (± 0.71%) 1.24s (± 0.55%) +0.01s (+ 0.57%) 1.23s 1.26s
Bind Time 0.68s (± 0.50%) 0.68s (± 1.20%) -0.00s (- 0.44%) 0.65s 0.69s
Check Time 4.61s (± 0.53%) 4.60s (± 0.41%) -0.01s (- 0.24%) 4.57s 4.65s
Emit Time 2.94s (± 1.16%) 2.96s (± 0.43%) +0.02s (+ 0.58%) 2.95s 3.01s
Total Time 9.47s (± 0.53%) 9.48s (± 0.24%) +0.01s (+ 0.10%) 9.42s 9.53s
material-ui - node (v10.16.3, x64)
Memory used 489,882k (± 0.01%) 489,823k (± 0.01%) -59k (- 0.01%) 489,653k 489,978k
Parse Time 2.06s (± 0.49%) 2.06s (± 0.31%) -0.01s (- 0.29%) 2.05s 2.08s
Bind Time 0.66s (± 1.02%) 0.66s (± 1.02%) 0.00s ( 0.00%) 0.64s 0.67s
Check Time 13.58s (± 0.88%) 13.59s (± 0.81%) +0.01s (+ 0.08%) 13.42s 13.83s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.30s (± 0.76%) 16.31s (± 0.68%) +0.01s (+ 0.06%) 16.15s 16.55s
Angular - node (v12.1.0, x64)
Memory used 322,792k (± 0.02%) 322,722k (± 0.03%) -69k (- 0.02%) 322,580k 322,932k
Parse Time 1.96s (± 0.81%) 1.96s (± 0.72%) +0.00s (+ 0.26%) 1.94s 2.01s
Bind Time 0.81s (± 0.87%) 0.81s (± 0.86%) +0.00s (+ 0.25%) 0.80s 0.83s
Check Time 4.91s (± 0.62%) 4.89s (± 0.57%) -0.02s (- 0.49%) 4.83s 4.98s
Emit Time 5.48s (± 0.51%) 5.51s (± 0.57%) +0.02s (+ 0.46%) 5.44s 5.57s
Total Time 13.17s (± 0.44%) 13.17s (± 0.42%) +0.01s (+ 0.05%) 13.06s 13.32s
Compiler-Unions - node (v12.1.0, x64)
Memory used 191,569k (± 0.06%) 191,492k (± 0.09%) -77k (- 0.04%) 191,041k 191,708k
Parse Time 0.77s (± 0.52%) 0.78s (± 0.77%) +0.00s (+ 0.52%) 0.76s 0.79s
Bind Time 0.49s (± 1.01%) 0.50s (± 0.89%) +0.01s (+ 1.41%) 0.49s 0.51s
Check Time 10.70s (± 0.48%) 10.86s (± 1.18%) +0.16s (+ 1.48%) 10.60s 11.12s
Emit Time 2.35s (± 1.13%) 2.36s (± 0.98%) +0.01s (+ 0.43%) 2.29s 2.41s
Total Time 14.32s (± 0.35%) 14.50s (± 0.97%) +0.18s (+ 1.26%) 14.23s 14.76s
Monaco - node (v12.1.0, x64)
Memory used 337,077k (± 0.02%) 337,084k (± 0.01%) +6k (+ 0.00%) 336,981k 337,199k
Parse Time 1.58s (± 0.57%) 1.59s (± 1.18%) +0.01s (+ 0.89%) 1.57s 1.64s
Bind Time 0.70s (± 0.52%) 0.71s (± 0.95%) +0.00s (+ 0.43%) 0.70s 0.72s
Check Time 4.91s (± 0.36%) 4.93s (± 0.70%) +0.02s (+ 0.41%) 4.87s 5.03s
Emit Time 2.86s (± 0.37%) 2.86s (± 0.82%) +0.00s (+ 0.14%) 2.81s 2.92s
Total Time 10.05s (± 0.20%) 10.09s (± 0.52%) +0.04s (+ 0.41%) 10.01s 10.22s
TFS - node (v12.1.0, x64)
Memory used 292,152k (± 0.02%) 292,159k (± 0.02%) +7k (+ 0.00%) 292,046k 292,279k
Parse Time 1.25s (± 0.66%) 1.25s (± 0.80%) 0.00s ( 0.00%) 1.22s 1.27s
Bind Time 0.65s (± 0.89%) 0.66s (± 0.91%) +0.01s (+ 0.92%) 0.65s 0.67s
Check Time 4.51s (± 0.42%) 4.52s (± 0.73%) +0.01s (+ 0.24%) 4.45s 4.61s
Emit Time 2.96s (± 1.15%) 2.97s (± 0.68%) +0.00s (+ 0.07%) 2.92s 3.02s
Total Time 9.38s (± 0.52%) 9.39s (± 0.52%) +0.02s (+ 0.19%) 9.31s 9.50s
material-ui - node (v12.1.0, x64)
Memory used 467,820k (± 0.06%) 467,657k (± 0.07%) -163k (- 0.03%) 466,747k 467,992k
Parse Time 2.07s (± 0.22%) 2.09s (± 0.50%) +0.01s (+ 0.72%) 2.07s 2.11s
Bind Time 0.64s (± 0.81%) 0.65s (± 0.86%) +0.01s (+ 1.09%) 0.64s 0.66s
Check Time 12.07s (± 0.75%) 12.21s (± 0.91%) +0.14s (+ 1.17%) 11.98s 12.45s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.78s (± 0.64%) 14.94s (± 0.75%) +0.16s (+ 1.08%) 14.70s 15.19s
Angular - node (v8.9.0, x64)
Memory used 347,657k (± 0.02%) 347,692k (± 0.02%) +35k (+ 0.01%) 347,489k 347,808k
Parse Time 2.51s (± 0.28%) 2.53s (± 1.06%) +0.02s (+ 0.92%) 2.50s 2.63s
Bind Time 0.86s (± 0.82%) 0.86s (± 0.95%) +0.00s (+ 0.12%) 0.85s 0.89s
Check Time 5.63s (± 0.77%) 5.64s (± 0.80%) +0.01s (+ 0.25%) 5.57s 5.79s
Emit Time 6.32s (± 1.19%) 6.43s (± 1.09%) +0.11s (+ 1.76%) 6.30s 6.56s
Total Time 15.31s (± 0.63%) 15.46s (± 0.45%) +0.14s (+ 0.93%) 15.31s 15.62s
Compiler-Unions - node (v8.9.0, x64)
Memory used 213,206k (± 0.02%) 213,175k (± 0.03%) -31k (- 0.01%) 212,975k 213,237k
Parse Time 0.95s (± 0.62%) 0.95s (± 0.49%) -0.00s (- 0.42%) 0.94s 0.96s
Bind Time 0.58s (± 1.12%) 0.58s (± 1.21%) +0.00s (+ 0.52%) 0.56s 0.59s
Check Time 14.79s (± 0.78%) 14.80s (± 1.01%) +0.01s (+ 0.06%) 14.48s 15.15s
Emit Time 2.74s (± 1.78%) 2.73s (± 2.54%) -0.01s (- 0.40%) 2.60s 2.91s
Total Time 19.06s (± 0.67%) 19.06s (± 1.01%) 0.00s ( 0.00%) 18.75s 19.47s
Monaco - node (v8.9.0, x64)
Memory used 358,817k (± 0.02%) 358,789k (± 0.02%) -29k (- 0.01%) 358,691k 358,967k
Parse Time 1.93s (± 0.40%) 1.93s (± 0.46%) +0.00s (+ 0.10%) 1.90s 1.94s
Bind Time 0.90s (± 0.75%) 0.91s (± 0.33%) +0.01s (+ 0.88%) 0.91s 0.92s
Check Time 5.68s (± 0.42%) 5.70s (± 0.45%) +0.01s (+ 0.25%) 5.66s 5.75s
Emit Time 3.41s (± 0.51%) 3.43s (± 0.57%) +0.01s (+ 0.41%) 3.38s 3.47s
Total Time 11.93s (± 0.35%) 11.97s (± 0.34%) +0.04s (+ 0.33%) 11.87s 12.04s
TFS - node (v8.9.0, x64)
Memory used 310,545k (± 0.02%) 310,520k (± 0.02%) -25k (- 0.01%) 310,402k 310,635k
Parse Time 1.57s (± 0.43%) 1.56s (± 0.31%) -0.00s (- 0.13%) 1.55s 1.57s
Bind Time 0.68s (± 0.73%) 0.69s (± 0.58%) +0.01s (+ 0.88%) 0.68s 0.70s
Check Time 5.32s (± 0.60%) 5.36s (± 0.61%) +0.04s (+ 0.71%) 5.26s 5.41s
Emit Time 2.96s (± 0.83%) 2.99s (± 1.01%) +0.03s (+ 0.98%) 2.91s 3.08s
Total Time 10.53s (± 0.44%) 10.60s (± 0.57%) +0.07s (+ 0.66%) 10.42s 10.74s
material-ui - node (v8.9.0, x64)
Memory used 496,998k (± 0.01%) 497,028k (± 0.01%) +30k (+ 0.01%) 496,940k 497,233k
Parse Time 2.48s (± 0.37%) 2.49s (± 0.53%) +0.02s (+ 0.61%) 2.47s 2.52s
Bind Time 0.81s (± 1.42%) 0.81s (± 1.09%) -0.01s (- 0.74%) 0.78s 0.82s
Check Time 18.32s (± 0.30%) 18.21s (± 0.71%) -0.11s (- 0.61%) 17.93s 18.62s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.62s (± 0.27%) 21.52s (± 0.62%) -0.10s (- 0.47%) 21.26s 21.96s
Angular - node (v8.9.0, x86)
Memory used 199,454k (± 0.03%) 199,420k (± 0.02%) -34k (- 0.02%) 199,317k 199,513k
Parse Time 2.42s (± 0.64%) 2.43s (± 0.53%) +0.01s (+ 0.33%) 2.40s 2.46s
Bind Time 1.02s (± 0.66%) 1.01s (± 0.66%) -0.01s (- 1.37%) 0.99s 1.02s
Check Time 5.10s (± 0.64%) 5.08s (± 0.63%) -0.02s (- 0.49%) 5.01s 5.17s
Emit Time 6.13s (± 0.80%) 6.10s (± 0.32%) -0.02s (- 0.39%) 6.05s 6.13s
Total Time 14.68s (± 0.47%) 14.62s (± 0.25%) -0.06s (- 0.38%) 14.53s 14.70s
Compiler-Unions - node (v8.9.0, x86)
Memory used 128,147k (± 0.04%) 128,123k (± 0.03%) -25k (- 0.02%) 128,039k 128,211k
Parse Time 0.96s (± 0.73%) 0.96s (± 0.69%) -0.00s (- 0.10%) 0.95s 0.98s
Bind Time 0.50s (± 0.89%) 0.49s (± 0.74%) -0.00s (- 0.80%) 0.49s 0.50s
Check Time 13.91s (± 0.33%) 13.93s (± 0.58%) +0.02s (+ 0.14%) 13.81s 14.19s
Emit Time 2.68s (± 1.90%) 2.63s (± 0.74%) -0.05s (- 1.98%) 2.59s 2.66s
Total Time 18.05s (± 0.38%) 18.01s (± 0.41%) -0.04s (- 0.22%) 17.89s 18.23s
Monaco - node (v8.9.0, x86)
Memory used 203,292k (± 0.01%) 203,261k (± 0.02%) -31k (- 0.02%) 203,160k 203,351k
Parse Time 1.98s (± 0.86%) 1.97s (± 0.84%) -0.00s (- 0.15%) 1.95s 2.03s
Bind Time 0.71s (± 0.84%) 0.71s (± 0.84%) 0.00s ( 0.00%) 0.70s 0.73s
Check Time 5.72s (± 1.77%) 5.79s (± 0.78%) +0.07s (+ 1.26%) 5.63s 5.85s
Emit Time 2.86s (± 4.23%) 2.79s (± 2.33%) -0.06s (- 2.17%) 2.72s 3.03s
Total Time 11.27s (± 0.42%) 11.27s (± 0.32%) +0.01s (+ 0.07%) 11.21s 11.36s
TFS - node (v8.9.0, x86)
Memory used 177,632k (± 0.03%) 177,692k (± 0.02%) +61k (+ 0.03%) 177,594k 177,759k
Parse Time 1.61s (± 0.75%) 1.60s (± 0.69%) -0.01s (- 0.37%) 1.58s 1.63s
Bind Time 0.65s (± 0.62%) 0.65s (± 0.85%) +0.00s (+ 0.46%) 0.64s 0.67s
Check Time 4.87s (± 0.82%) 4.90s (± 0.61%) +0.02s (+ 0.51%) 4.84s 4.96s
Emit Time 2.85s (± 1.05%) 2.84s (± 1.65%) -0.01s (- 0.39%) 2.76s 2.96s
Total Time 9.98s (± 0.62%) 9.99s (± 0.61%) +0.01s (+ 0.09%) 9.88s 10.15s
material-ui - node (v8.9.0, x86)
Memory used 279,876k (± 0.02%) 279,869k (± 0.02%) -7k (- 0.00%) 279,797k 279,990k
Parse Time 2.54s (± 0.72%) 2.54s (± 0.72%) +0.00s (+ 0.00%) 2.51s 2.59s
Bind Time 0.72s (± 4.77%) 0.79s (± 6.39%) +0.07s (+10.01%) 0.69s 0.85s
Check Time 16.57s (± 0.80%) 16.52s (± 0.57%) -0.05s (- 0.30%) 16.42s 16.78s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.84s (± 0.80%) 19.86s (± 0.43%) +0.02s (+ 0.12%) 19.70s 20.06s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-197-generic
Architecturex64
Available Memory16 GB
Available Memory10 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)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v8.9.0, x64)
  • Compiler-Unions - 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 41972 10
Baseline master 10

@ahejlsberg ahejlsberg merged commit bec8071 into master Dec 16, 2020
@ahejlsberg ahejlsberg deleted the fix41931 branch December 16, 2020 03:08
@mkantor mkantor mentioned this pull request Dec 22, 2020
Closed
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

@RyanCavanaugh RyanCavanaugh Awaiting requested review from RyanCavanaugh

Assignees

@ahejlsberg ahejlsberg

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Typescript 4.1+: Nested template type deduction error
3 participants
@ahejlsberg @typescript-bot @weswigham