Skip to content

Error on excessive relation complexity - #55851

Merged
Daniel Rosenwasser (DanielRosenwasser) merged 6 commits into
mainfrom
fix55630
Sep 26, 2023
Merged

Error on excessive relation complexity#55851
Daniel Rosenwasser (DanielRosenwasser) merged 6 commits into
mainfrom
fix55630

Conversation

@ahejlsberg

@ahejlsbergAnders Hejlsberg (ahejlsberg) commented Sep 25, 2023

Copy link
Copy Markdown
Member

With this PR we now error when computing a relation is excessively complex. Specifically, when computing a relation results in adding more relation cache entries than 1/8th of the current capacity of that relation, we issue an error. Relation caches use JavaScript maps, which in Node.js are limited to 2^24 (~16M) entries.

An example of a relation computation that is excessively complex is the following:

typeDigits='0'|'1'|'2'|'3'|'4'|'5'|'6'|'7'|'8'|'9';typeT1= `${Digits}${Digits}${Digits}${Digits}` |undefined;typeT2={a: string}|{b: number};functionf1(x: T1|null,y: T1&T2){x=y;// Excessive complexity error}

Above, T1 is a union with 10,001 members, and T2 normalizes to a union with 20,002 members.

This PR further implements an optimization outlined here. For example, the following example doesn't cause an excessive complexity error because we now have a fast path when relating a union resulting from normalizing an intersection of large unions to one of those same unions.

functionf2(x: T1,y: T1&T2){x=y;// Ok}

Fixes#55630.

# Conflicts:
#	src/compiler/diagnosticMessages.json
@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 25, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the tsc-only perf test suite on this PR at 89f5d10. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 25, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based top-repos suite on this PR at 89f5d10. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 25, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based user code test suite on this PR at 89f5d10. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 25, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the parallelized Definitely Typed test suite on this PR at 89f5d10. You can monitor the build here.

Update: The results are in!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Angular - node (v18.15.0, x64)
Memory used294,958k (± 0.01%)294,960k (± 0.01%)~294,938k294,997kp=1.000 n=6
Parse Time2.62s (± 0.31%)2.63s (± 0.47%)~2.61s2.64sp=0.730 n=6
Bind Time0.84s (± 1.17%)0.84s (± 0.97%)~0.83s0.85sp=0.394 n=6
Check Time8.05s (± 0.46%)8.06s (± 0.34%)~8.03s8.10sp=0.629 n=6
Emit Time7.05s (± 0.30%)7.03s (± 0.16%)~7.01s7.04sp=0.061 n=6
Total Time18.56s (± 0.22%)18.55s (± 0.16%)~18.50s18.58sp=1.000 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used192,115k (± 1.24%)193,628k (± 1.64%)~190,726k196,592kp=0.423 n=6
Parse Time1.34s (± 0.77%)1.34s (± 1.05%)~1.32s1.36sp=0.738 n=6
Bind Time0.73s (± 0.00%)0.73s (± 0.00%)~0.73s0.73sp=1.000 n=6
Check Time9.11s (± 0.54%)9.17s (± 0.46%)~9.12s9.24sp=0.078 n=6
Emit Time2.64s (± 0.44%)2.63s (± 0.83%)~2.60s2.66sp=0.192 n=6
Total Time13.81s (± 0.45%)13.87s (± 0.30%)~13.79s13.91sp=0.092 n=6
Monaco - node (v18.15.0, x64)
Memory used347,217k (± 0.00%)347,209k (± 0.01%)~347,183k347,239kp=0.575 n=6
Parse Time2.46s (± 0.43%)2.44s (± 0.34%)~2.44s2.46sp=0.109 n=6
Bind Time0.94s (± 0.43%)0.94s (± 0.00%)~0.94s0.94sp=0.405 n=6
Check Time6.88s (± 0.41%)6.88s (± 0.58%)~6.84s6.94sp=1.000 n=6
Emit Time4.02s (± 0.41%)4.02s (± 0.34%)~4.01s4.05sp=0.505 n=6
Total Time14.28s (± 0.28%)14.29s (± 0.19%)~14.26s14.33sp=0.808 n=6
TFS - node (v18.15.0, x64)
Memory used302,484k (± 0.00%)302,554k (± 0.01%)+71k (+ 0.02%)302,498k302,598kp=0.008 n=6
Parse Time2.01s (± 0.41%)2.00s (± 0.63%)-0.01s (- 0.74%)1.98s2.01sp=0.038 n=6
Bind Time1.00s (± 1.17%)1.00s (± 0.83%)~0.99s1.01sp=0.555 n=6
Check Time6.25s (± 0.28%)6.25s (± 0.48%)~6.22s6.30sp=0.870 n=6
Emit Time3.54s (± 1.34%)3.53s (± 0.83%)~3.50s3.57sp=1.000 n=6
Total Time12.81s (± 0.38%)12.80s (± 0.32%)~12.74s12.86sp=0.630 n=6
material-ui - node (v18.15.0, x64)
Memory used470,448k (± 0.00%)470,426k (± 0.01%)~470,382k470,463kp=0.128 n=6
Parse Time2.57s (± 0.58%)2.56s (± 0.64%)~2.55s2.59sp=0.314 n=6
Bind Time1.00s (± 0.75%)0.99s (± 1.04%)~0.98s1.01sp=0.155 n=6
Check Time16.56s (± 0.59%)16.57s (± 0.33%)~16.51s16.67sp=0.872 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time20.13s (± 0.56%)20.13s (± 0.24%)~20.08s20.21sp=0.748 n=6
xstate - node (v18.15.0, x64)
Memory used512,548k (± 0.01%)512,514k (± 0.02%)~512,399k512,749kp=0.298 n=6
Parse Time3.27s (± 0.17%)3.27s (± 0.23%)~3.26s3.28sp=0.476 n=6
Bind Time1.55s (± 0.33%)1.55s (± 0.53%)~1.54s1.56sp=0.929 n=6
Check Time2.83s (± 0.69%)2.85s (± 1.15%)~2.80s2.90sp=0.226 n=6
Emit Time0.08s (± 4.99%)0.08s (± 0.00%)~0.08s0.08sp=0.405 n=6
Total Time7.71s (± 0.25%)7.74s (± 0.43%)~7.69s7.79sp=0.196 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the user test suite comparing main and refs/pull/55851/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 2 instances of "Package install failed"

Otherwise...

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the top-repos suite comparing main and refs/pull/55851/merge:

Everything looks good!

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Anders Hejlsberg (@ahejlsberg), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@gabritto

Copy link
Copy Markdown
Member

On PR #50329, Wesley Wigham (@weswigham) had proposed a more general form of the optimization found on this PR. What's the reasoning behind picking this simpler form of the optimization instead?

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

What's the reasoning behind picking this simpler form of the optimization instead?

I hadn't seen #50329, but certainly the optimization in this PR removes far more work when the fast path is available. There's no reason the two couldn't co-exist.

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 26, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based user code test suite on this PR at be68f32. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 26, 2023

Copy link
Copy Markdown
Contributor

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 26, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the tsc-only perf test suite on this PR at be68f32. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 26, 2023

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the diff-based top-repos suite on this PR at be68f32. You can monitor the build here.

Update: The results are in!

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Angular - node (v18.15.0, x64)
Memory used294,994k (± 0.01%)294,988k (± 0.01%)~294,945k295,050kp=0.873 n=6
Parse Time2.64s (± 0.73%)2.63s (± 0.65%)~2.60s2.65sp=0.216 n=6
Bind Time0.84s (± 0.97%)0.84s (± 1.17%)~0.83s0.85sp=0.394 n=6
Check Time8.06s (± 0.21%)8.06s (± 0.35%)~8.02s8.09sp=0.935 n=6
Emit Time7.04s (± 0.15%)7.04s (± 0.25%)~7.02s7.07sp=1.000 n=6
Total Time18.58s (± 0.13%)18.57s (± 0.24%)~18.52s18.65sp=0.517 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used190,707k (± 0.01%)192,093k (± 1.25%)~190,656k196,506kp=1.000 n=6
Parse Time1.35s (± 0.56%)1.35s (± 0.47%)~1.34s1.36sp=0.718 n=6
Bind Time0.73s (± 0.56%)0.73s (± 0.00%)~0.73s0.73sp=0.405 n=6
Check Time9.15s (± 0.63%)9.16s (± 0.31%)~9.12s9.20sp=0.468 n=6
Emit Time2.63s (± 0.50%)2.64s (± 0.59%)~2.62s2.66sp=0.452 n=6
Total Time13.86s (± 0.32%)13.88s (± 0.22%)~13.84s13.93sp=0.370 n=6
Monaco - node (v18.15.0, x64)
Memory used347,227k (± 0.01%)347,240k (± 0.00%)~347,213k347,257kp=0.378 n=6
Parse Time2.46s (± 0.56%)2.45s (± 0.26%)~2.44s2.46sp=0.362 n=6
Bind Time0.94s (± 0.43%)0.94s (± 0.88%)~0.94s0.96sp=0.527 n=6
Check Time6.87s (± 0.28%)6.88s (± 0.20%)~6.86s6.90sp=0.167 n=6
Emit Time4.03s (± 0.43%)4.03s (± 0.29%)~4.02s4.05sp=1.000 n=6
Total Time14.29s (± 0.27%)14.31s (± 0.17%)~14.27s14.34sp=0.332 n=6
TFS - node (v18.15.0, x64)
Memory used302,503k (± 0.01%)302,550k (± 0.01%)~302,491k302,613kp=0.065 n=6
Parse Time2.01s (± 0.86%)1.99s (± 0.49%)~1.98s2.01sp=0.072 n=6
Bind Time1.01s (± 0.97%)1.01s (± 1.20%)~0.99s1.02sp=0.865 n=6
Check Time6.24s (± 0.19%)6.26s (± 0.55%)~6.20s6.30sp=0.256 n=6
Emit Time3.52s (± 0.53%)3.52s (± 0.45%)~3.50s3.55sp=0.490 n=6
Total Time12.77s (± 0.23%)12.78s (± 0.23%)~12.73s12.81sp=0.413 n=6
material-ui - node (v18.15.0, x64)
Memory used470,468k (± 0.00%)470,493k (± 0.01%)~470,456k470,519kp=0.173 n=6
Parse Time2.56s (± 0.48%)2.57s (± 0.59%)~2.55s2.59sp=1.000 n=6
Bind Time0.99s (± 1.18%)1.00s (± 0.84%)~0.98s1.00sp=0.555 n=6
Check Time16.62s (± 0.43%)16.60s (± 0.33%)~16.51s16.66sp=0.627 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time20.18s (± 0.35%)20.17s (± 0.32%)~20.07s20.24sp=0.747 n=6
xstate - node (v18.15.0, x64)
Memory used512,559k (± 0.02%)512,535k (± 0.01%)~512,435k512,635kp=0.748 n=6
Parse Time3.27s (± 0.45%)3.27s (± 0.32%)~3.25s3.28sp=1.000 n=6
Bind Time1.55s (± 0.33%)1.55s (± 0.53%)~1.54s1.56sp=0.140 n=6
Check Time2.83s (± 0.62%)2.84s (± 1.16%)~2.80s2.89sp=0.466 n=6
Emit Time0.08s (± 5.21%)0.08s (± 0.00%)~0.08s0.08sp=0.405 n=6
Total Time7.71s (± 0.37%)7.73s (± 0.47%)~7.68s7.78sp=0.467 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@typescript-bot

Copy link
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg) Here are the results of running the top-repos suite comparing main and refs/pull/55851/merge:

Everything looks good!

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: TeamFor Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler crash with RangeError: Map maximum size exceeded

5 participants

@ahejlsberg@typescript-bot@gabritto@sandersn@DanielRosenwasser