Skip to content

Cache results of expensive repetitive type operations #49760

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 1 commit into from
Jul 5, 2022
Merged

Cache results of expensive repetitive type operations #49760

merged 1 commit into from
Jul 5, 2022

Conversation

ahejlsberg
Copy link
Member

Fixes #49622.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Jul 2, 2022
@ahejlsberg
Copy link
Member Author

@typescript-bot perf test faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jul 2, 2022 •
edited
Loading

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

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 - main..49760

Metric main 49760 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 335,314k (± 0.01%) 335,336k (± 0.01%) +22k (+ 0.01%) 335,268k 335,405k
Parse Time 2.08s (± 0.51%) 2.06s (± 0.65%) -0.01s (- 0.53%) 2.04s 2.10s
Bind Time 0.90s (± 0.72%) 0.90s (± 0.52%) -0.00s (- 0.22%) 0.89s 0.91s
Check Time 5.74s (± 0.54%) 5.77s (± 0.52%) +0.03s (+ 0.58%) 5.71s 5.85s
Emit Time 6.40s (± 0.86%) 6.42s (± 0.58%) +0.03s (+ 0.44%) 6.36s 6.51s
Total Time 15.11s (± 0.54%) 15.16s (± 0.32%) +0.05s (+ 0.35%) 15.07s 15.28s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,718k (± 0.02%) 192,812k (± 0.01%) +94k (+ 0.05%) 192,759k 192,861k
Parse Time 0.85s (± 0.76%) 0.85s (± 0.83%) -0.00s (- 0.35%) 0.84s 0.87s
Bind Time 0.57s (± 1.17%) 0.57s (± 1.17%) -0.00s (- 0.00%) 0.56s 0.59s
Check Time 7.65s (± 0.56%) 6.75s (± 0.93%) 🟩-0.91s (-11.85%) 6.62s 6.88s
Emit Time 2.51s (± 0.71%) 2.51s (± 0.59%) +0.00s (+ 0.16%) 2.47s 2.54s
Total Time 11.59s (± 0.46%) 10.68s (± 0.63%) 🟩-0.91s (- 7.82%) 10.53s 10.79s
Monaco - node (v14.15.1, x64)
Memory used 325,665k (± 0.01%) 325,670k (± 0.01%) +5k (+ 0.00%) 325,643k 325,724k
Parse Time 1.58s (± 0.87%) 1.58s (± 0.52%) +0.00s (+ 0.25%) 1.56s 1.60s
Bind Time 0.79s (± 0.59%) 0.79s (± 0.38%) +0.00s (+ 0.25%) 0.79s 0.80s
Check Time 5.69s (± 0.57%) 5.68s (± 0.59%) -0.01s (- 0.21%) 5.62s 5.77s
Emit Time 3.36s (± 0.63%) 3.36s (± 0.84%) -0.00s (- 0.09%) 3.31s 3.44s
Total Time 11.43s (± 0.33%) 11.42s (± 0.50%) -0.01s (- 0.12%) 11.32s 11.57s
TFS - node (v14.15.1, x64)
Memory used 288,787k (± 0.01%) 288,760k (± 0.00%) -28k (- 0.01%) 288,738k 288,791k
Parse Time 1.35s (± 1.56%) 1.34s (± 1.18%) -0.01s (- 0.52%) 1.30s 1.37s
Bind Time 0.76s (± 4.89%) 0.76s (± 4.71%) +0.00s (+ 0.39%) 0.73s 0.86s
Check Time 5.31s (± 0.34%) 5.31s (± 0.53%) +0.00s (+ 0.08%) 5.26s 5.36s
Emit Time 3.58s (± 2.04%) 3.58s (± 2.11%) +0.00s (+ 0.06%) 3.46s 3.73s
Total Time 11.00s (± 0.67%) 11.00s (± 0.98%) -0.00s (- 0.02%) 10.83s 11.20s
material-ui - node (v14.15.1, x64)
Memory used 446,670k (± 0.01%) 446,668k (± 0.00%) -1k (- 0.00%) 446,626k 446,709k
Parse Time 1.88s (± 0.53%) 1.88s (± 0.32%) -0.00s (- 0.27%) 1.86s 1.89s
Bind Time 0.73s (± 0.96%) 0.73s (± 1.75%) +0.00s (+ 0.41%) 0.71s 0.78s
Check Time 13.13s (± 0.85%) 13.21s (± 0.69%) +0.08s (+ 0.59%) 13.06s 13.44s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.74s (± 0.76%) 15.81s (± 0.62%) +0.07s (+ 0.46%) 15.65s 16.04s
xstate - node (v14.15.1, x64)
Memory used 541,129k (± 0.00%) 541,181k (± 0.00%) +53k (+ 0.01%) 541,143k 541,238k
Parse Time 2.62s (± 0.74%) 2.61s (± 0.60%) -0.00s (- 0.15%) 2.58s 2.66s
Bind Time 1.16s (± 0.91%) 1.15s (± 0.71%) -0.00s (- 0.35%) 1.13s 1.17s
Check Time 1.55s (± 0.81%) 1.54s (± 0.55%) -0.01s (- 0.32%) 1.52s 1.56s
Emit Time 0.07s (± 4.95%) 0.07s (± 4.13%) 🟩-0.00s (- 4.00%) 0.07s 0.08s
Total Time 5.39s (± 0.42%) 5.38s (± 0.39%) -0.02s (- 0.32%) 5.33s 5.43s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 49760 10
Baseline main 10

Developer Information:

Download Benchmark

@ahejlsberg
Copy link
Member Author

Nice 12% improvement in check time for compiler-unions test.

jakebailey
jakebailey reviewed Jul 3, 2022
View reviewed changes
src/compiler/checker.ts
@@ -774,6 +774,7 @@ namespace ts {
const stringMappingTypes = new Map<string, StringMappingType>();
const substitutionTypes = new Map<string, SubstitutionType>();
const subtypeReductionCache = new Map<string, Type[]>();
const cachedTypes = new Map<string, Type>();
Copy link
Member

Choose a reason for hiding this comment

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

Why one cache for multiple calls rather than one per? I would have thought that would be more efficient because you don't have to make special keys (and could just use the type ids as keys) and for locality reasons, so I'm curious is there's some perf thing I'm missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

In two of the three cases we have to create composite keys anyways, so just by making sure the keys don't overlap we can share the same cache and access helpers. In the third case we could just go by type id, but would save us very little time and could add more lines of code.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed the composite key.

@ahejlsberg ahejlsberg merged commit 2f26088 into main Jul 5, 2022
@ahejlsberg ahejlsberg deleted the fix49622 branch July 5, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakebailey jakebailey jakebailey left review comments

@RyanCavanaugh RyanCavanaugh RyanCavanaugh approved these changes

@aroramrinaal aroramrinaal aroramrinaal approved these changes

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.

Investigate 4.8 perf regression from "Use NonNullable<T> in more scenarios"
5 participants
@ahejlsberg @typescript-bot @jakebailey @RyanCavanaugh @aroramrinaal