Skip to content

Optimize union type creation #53771

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 4 commits into from
Apr 14, 2023
Merged

Optimize union type creation #53771

merged 4 commits into from
Apr 14, 2023

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Apr 14, 2023

This PR implements two optimizations to our union type creation logic:

  • When resolving the type Record<A, B>[A], where A and B are large union types, we end up creating a union type from a list of types B, B, ..., B, where B is repeated as many times as the size of A. The union type creation logic collects the constituent type list by "flattening" each B, which ends up generating a lot of useless work. We now detect that we've already processed B and ignore all but the first occurrence.
  • We now cache the results of unioning a pair of types, where one or both are union types, by a key constructed from the two type identities. Previously, we'd first flatten all of the (non-union) types into a combined set, which is a lot more work for large unions. This in particular helps for repeated creations of U | undefined where U is some large union type.

The check time for the example in #53761 effectively drops to zero with these optimizations.

Fixes #53761.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 14, 2023
@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 46d1c4e. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

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

Update: The results are in!

@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 14, 2023
@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/53771/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..53771

Metric main 53771 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 364,558k (± 0.01%) 364,581k (± 0.01%) ~ 364,534k 364,605k p=0.199 n=6
Parse Time 3.54s (± 0.49%) 3.56s (± 0.66%) ~ 3.53s 3.59s p=0.371 n=6
Bind Time 1.17s (± 0.47%) 1.18s (± 0.76%) ~ 1.17s 1.19s p=0.341 n=6
Check Time 9.54s (± 0.49%) 9.50s (± 0.39%) ~ 9.46s 9.56s p=0.195 n=6
Emit Time 7.97s (± 0.64%) 7.96s (± 0.44%) ~ 7.92s 8.00s p=0.807 n=6
Total Time 22.22s (± 0.22%) 22.20s (± 0.32%) ~ 22.11s 22.27s p=0.572 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 192,423k (± 0.02%) 192,605k (± 0.04%) +182k (+ 0.09%) 192,455k 192,680k p=0.013 n=6
Parse Time 1.59s (± 0.79%) 1.59s (± 0.65%) ~ 1.57s 1.60s p=0.560 n=6
Bind Time 0.83s (± 0.66%) 0.82s (± 0.63%) ~ 0.82s 0.83s p=0.640 n=6
Check Time 10.36s (± 0.37%) 10.21s (± 0.72%) -0.15s (- 1.46%) 10.15s 10.30s p=0.005 n=6
Emit Time 3.00s (± 1.48%) 2.98s (± 0.84%) ~ 2.96s 3.02s p=0.627 n=6
Total Time 15.78s (± 0.52%) 15.61s (± 0.48%) -0.17s (- 1.07%) 15.51s 15.72s p=0.010 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,774k (± 0.00%) 345,784k (± 0.00%) ~ 345,771k 345,809k p=0.261 n=6
Parse Time 2.72s (± 0.31%) 2.72s (± 0.49%) ~ 2.70s 2.74s p=0.280 n=6
Bind Time 1.08s (± 1.23%) 1.08s (± 0.50%) ~ 1.08s 1.09s p=0.663 n=6
Check Time 7.81s (± 0.57%) 7.83s (± 0.53%) ~ 7.79s 7.88s p=0.454 n=6
Emit Time 4.46s (± 1.36%) 4.48s (± 1.03%) ~ 4.44s 4.53s p=0.514 n=6
Total Time 16.07s (± 0.44%) 16.12s (± 0.40%) ~ 16.04s 16.19s p=0.198 n=6
TFS - node (v16.17.1, x64)
Memory used 300,049k (± 0.00%) 300,047k (± 0.01%) ~ 300,024k 300,082k p=0.936 n=6
Parse Time 2.17s (± 0.35%) 2.16s (± 0.51%) ~ 2.15s 2.18s p=0.068 n=6
Bind Time 1.23s (± 0.33%) 1.24s (± 1.22%) ~ 1.22s 1.26s p=0.498 n=6
Check Time 7.22s (± 0.20%) 7.18s (± 0.50%) ~ 7.13s 7.23s p=0.106 n=6
Emit Time 4.35s (± 0.86%) 4.33s (± 0.63%) ~ 4.30s 4.37s p=0.292 n=6
Total Time 14.97s (± 0.21%) 14.91s (± 0.39%) ~ 14.84s 15.00s p=0.107 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,359k (± 0.01%) 481,107k (± 0.00%) +1,749k (+ 0.36%) 481,092k 481,128k p=0.005 n=6
Parse Time 3.24s (± 0.16%) 3.23s (± 0.47%) ~ 3.22s 3.26s p=0.198 n=6
Bind Time 0.95s (± 0.79%) 0.95s (± 0.54%) ~ 0.94s 0.95s p=0.241 n=6
Check Time 18.00s (± 0.82%) 17.78s (± 0.22%) -0.21s (- 1.18%) 17.71s 17.82s p=0.005 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.19s (± 0.68%) 21.97s (± 0.20%) -0.22s (- 1.00%) 21.89s 22.01s p=0.005 n=6
xstate - node (v16.17.1, x64)
Memory used 559,387k (± 0.02%) 559,338k (± 0.01%) ~ 559,297k 559,408k p=0.689 n=6
Parse Time 3.99s (± 0.19%) 3.98s (± 0.49%) ~ 3.95s 4.00s p=0.871 n=6
Bind Time 1.75s (± 0.36%) 1.74s (± 0.43%) ~ 1.73s 1.75s p=0.081 n=6
Check Time 3.06s (± 0.91%) 3.03s (± 0.46%) -0.04s (- 1.25%) 3.00s 3.04s p=0.004 n=6
Emit Time 0.10s (± 5.34%) 0.09s (± 0.00%) 🟩-0.01s (- 6.90%) 0.09s 0.09s p=0.025 n=6
Total Time 8.90s (± 0.24%) 8.84s (± 0.30%) -0.06s (- 0.69%) 8.81s 8.88s p=0.006 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 53771 6
Baseline main 6

Developer Information:

Download Benchmark

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Very nice!

@typescript-bot
Copy link
Collaborator

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/53771/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

@jakebailey
Copy link
Member

@typescript-bot perf test this faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 14, 2023

Heya @jakebailey, I've started to run the abridged perf test suite on this PR at 14f30d3. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..53771

Metric main 53771 Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 364,587k (± 0.00%) 364,606k (± 0.01%) ~ 364,542k 364,631k p=0.065 n=6
Parse Time 3.54s (± 0.46%) 3.55s (± 0.49%) ~ 3.54s 3.58s p=0.210 n=6
Bind Time 1.17s (± 1.04%) 1.18s (± 0.76%) ~ 1.17s 1.19s p=0.445 n=6
Check Time 9.52s (± 0.57%) 9.49s (± 0.62%) ~ 9.40s 9.56s p=0.872 n=6
Emit Time 7.96s (± 0.31%) 7.94s (± 0.66%) ~ 7.88s 8.03s p=0.467 n=6
Total Time 22.20s (± 0.27%) 22.17s (± 0.43%) ~ 22.06s 22.27s p=0.689 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 192,362k (± 0.01%) 192,536k (± 0.02%) +174k (+ 0.09%) 192,479k 192,582k p=0.005 n=6
Parse Time 1.59s (± 0.83%) 1.58s (± 0.96%) ~ 1.56s 1.60s p=0.511 n=6
Bind Time 0.82s (± 0.63%) 0.82s (± 0.63%) ~ 0.82s 0.83s p=1.000 n=6
Check Time 10.36s (± 0.48%) 10.27s (± 0.64%) -0.08s (- 0.79%) 10.19s 10.39s p=0.037 n=6
Emit Time 3.00s (± 0.63%) 3.00s (± 1.02%) ~ 2.95s 3.03s p=1.000 n=6
Total Time 15.77s (± 0.35%) 15.69s (± 0.55%) ~ 15.60s 15.84s p=0.065 n=6
Monaco - node (v16.17.1, x64)
Memory used 345,772k (± 0.00%) 345,785k (± 0.01%) ~ 345,763k 345,808k p=0.230 n=6
Parse Time 2.72s (± 0.49%) 2.72s (± 0.68%) ~ 2.71s 2.76s p=0.732 n=6
Bind Time 1.08s (± 0.48%) 1.08s (± 1.01%) ~ 1.06s 1.09s p=0.784 n=6
Check Time 7.82s (± 0.76%) 7.83s (± 0.41%) ~ 7.80s 7.89s p=0.421 n=6
Emit Time 4.47s (± 0.61%) 4.46s (± 0.36%) ~ 4.44s 4.48s p=0.458 n=6
Total Time 16.10s (± 0.51%) 16.10s (± 0.22%) ~ 16.05s 16.15s p=0.571 n=6
TFS - node (v16.17.1, x64)
Memory used 300,041k (± 0.01%) 300,049k (± 0.01%) ~ 300,028k 300,085k p=0.810 n=6
Parse Time 2.16s (± 0.38%) 2.16s (± 0.35%) ~ 2.15s 2.17s p=0.729 n=6
Bind Time 1.23s (± 1.03%) 1.24s (± 0.85%) ~ 1.22s 1.25s p=0.456 n=6
Check Time 7.22s (± 0.67%) 7.18s (± 0.25%) ~ 7.15s 7.20s p=0.169 n=6
Emit Time 4.32s (± 0.27%) 4.35s (± 1.00%) ~ 4.31s 4.41s p=0.460 n=6
Total Time 14.94s (± 0.31%) 14.93s (± 0.40%) ~ 14.85s 15.00s p=0.686 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,399k (± 0.01%) 481,114k (± 0.00%) +1,715k (+ 0.36%) 481,092k 481,148k p=0.005 n=6
Parse Time 3.24s (± 0.42%) 3.25s (± 0.36%) ~ 3.23s 3.26s p=0.562 n=6
Bind Time 0.95s (± 0.67%) 0.95s (± 0.79%) ~ 0.94s 0.96s p=0.718 n=6
Check Time 17.97s (± 0.34%) 17.80s (± 0.38%) -0.17s (- 0.95%) 17.73s 17.90s p=0.008 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 22.17s (± 0.32%) 22.00s (± 0.32%) -0.16s (- 0.74%) 21.91s 22.09s p=0.013 n=6
xstate - node (v16.17.1, x64)
Memory used 559,327k (± 0.03%) 559,363k (± 0.02%) ~ 559,254k 559,487k p=0.471 n=6
Parse Time 3.99s (± 0.49%) 3.99s (± 0.20%) ~ 3.98s 4.00s p=1.000 n=6
Bind Time 1.75s (± 0.30%) 1.75s (± 0.00%) ~ 1.75s 1.75s p=0.174 n=6
Check Time 3.06s (± 0.27%) 3.03s (± 0.84%) ~ 3.01s 3.08s p=0.063 n=6
Emit Time 0.10s (± 5.34%) 0.09s (± 0.00%) 🟩-0.01s (- 6.90%) 0.09s 0.09s p=0.025 n=6
Total Time 8.89s (± 0.18%) 8.86s (± 0.31%) -0.03s (- 0.37%) 8.83s 8.91s p=0.036 n=6
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 53771 6
Baseline main 6

Developer Information:

Download Benchmark

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Approved x2 😄

@ahejlsberg ahejlsberg merged commit b798e6b into main Apr 14, 2023
@ahejlsberg ahejlsberg deleted the fix53761 branch April 14, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slow typings related to large enums
3 participants