GeezerVison is showing github.com — Cache results of isGenericObjectType and isGenericIndexType by ahejlsberg · Pull Request #36622 · microsoft/TypeScript
Conversation
With this PR we cache the results of isGenericObjectType and isGenericIndexType for unions and intersections. This improves total compile time of the repro in #36564 by about 6.5%.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
amcasey, dragomirtitian, MKRhere, osnoser1, and jpdenford reacted with hooray emoji
AviVahl reacted with rocket emoji
All reactions
-
🎉
5 reactions
-
🚀
1 reaction
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!
Here they are:
Comparison Report - master..36622
| Metric |
master |
36622 |
Delta |
Best |
Worst |
| Angular - node (v10.16.3, x64) |
|---|
| Memory used |
356,893k (± 0.02%) |
358,562k (± 0.02%) |
+1,669k (+ 0.47%) |
358,382k |
358,699k |
| Parse Time |
1.61s (± 0.52%) |
1.62s (± 0.63%) |
+0.01s (+ 0.50%) |
1.60s |
1.65s |
| Bind Time |
0.89s (± 1.06%) |
0.88s (± 0.92%) |
-0.01s (- 0.79%) |
0.86s |
0.90s |
| Check Time |
4.67s (± 0.67%) |
4.65s (± 0.60%) |
-0.02s (- 0.43%) |
4.60s |
4.72s |
| Emit Time |
5.21s (± 0.63%) |
5.20s (± 0.75%) |
-0.01s (- 0.23%) |
5.12s |
5.30s |
| Total Time |
12.38s (± 0.49%) |
12.35s (± 0.48%) |
-0.03s (- 0.22%) |
12.23s |
12.50s |
| Monaco - node (v10.16.3, x64) |
|---|
| Memory used |
364,574k (± 0.02%) |
364,633k (± 0.01%) |
+60k (+ 0.02%) |
364,537k |
364,767k |
| Parse Time |
1.24s (± 0.81%) |
1.25s (± 0.52%) |
+0.00s (+ 0.32%) |
1.24s |
1.27s |
| Bind Time |
0.77s (± 0.29%) |
0.77s (± 0.44%) |
+0.00s (+ 0.26%) |
0.77s |
0.78s |
| Check Time |
4.69s (± 0.62%) |
4.68s (± 0.38%) |
-0.00s (- 0.04%) |
4.66s |
4.74s |
| Emit Time |
2.90s (± 0.98%) |
2.89s (± 0.52%) |
-0.01s (- 0.45%) |
2.85s |
2.92s |
| Total Time |
9.61s (± 0.53%) |
9.60s (± 0.32%) |
-0.01s (- 0.09%) |
9.55s |
9.68s |
| TFS - node (v10.16.3, x64) |
|---|
| Memory used |
324,176k (± 0.02%) |
324,221k (± 0.08%) |
+45k (+ 0.01%) |
323,972k |
325,262k |
| Parse Time |
0.94s (± 0.59%) |
0.94s (± 0.53%) |
+0.00s (+ 0.32%) |
0.94s |
0.96s |
| Bind Time |
0.75s (± 0.80%) |
0.74s (± 1.38%) |
-0.01s (- 0.67%) |
0.71s |
0.75s |
| Check Time |
4.21s (± 0.48%) |
4.22s (± 0.48%) |
+0.01s (+ 0.21%) |
4.18s |
4.27s |
| Emit Time |
3.01s (± 0.87%) |
3.01s (± 1.26%) |
+0.00s (+ 0.03%) |
2.92s |
3.08s |
| Total Time |
8.90s (± 0.41%) |
8.91s (± 0.39%) |
+0.01s (+ 0.10%) |
8.81s |
8.98s |
| Angular - node (v12.1.0, x64) |
|---|
| Memory used |
332,682k (± 0.02%) |
334,173k (± 0.08%) |
+1,491k (+ 0.45%) |
333,192k |
334,390k |
| Parse Time |
1.55s (± 0.44%) |
1.57s (± 0.82%) |
+0.01s (+ 0.84%) |
1.55s |
1.60s |
| Bind Time |
0.87s (± 1.07%) |
0.87s (± 0.42%) |
+0.01s (+ 0.69%) |
0.87s |
0.88s |
| Check Time |
4.59s (± 0.49%) |
4.59s (± 0.68%) |
-0.00s (- 0.04%) |
4.52s |
4.68s |
| Emit Time |
5.40s (± 0.79%) |
5.43s (± 1.24%) |
+0.03s (+ 0.57%) |
5.29s |
5.62s |
| Total Time |
12.41s (± 0.51%) |
12.45s (± 0.77%) |
+0.05s (+ 0.37%) |
12.26s |
12.76s |
| Monaco - node (v12.1.0, x64) |
|---|
| Memory used |
344,511k (± 0.01%) |
344,539k (± 0.02%) |
+28k (+ 0.01%) |
344,408k |
344,696k |
| Parse Time |
1.21s (± 0.73%) |
1.21s (± 0.70%) |
-0.00s (- 0.25%) |
1.19s |
1.23s |
| Bind Time |
0.75s (± 0.86%) |
0.75s (± 0.87%) |
-0.00s (- 0.53%) |
0.74s |
0.76s |
| Check Time |
4.56s (± 0.61%) |
4.53s (± 0.30%) |
-0.03s (- 0.64%) |
4.50s |
4.56s |
| Emit Time |
2.97s (± 0.65%) |
2.96s (± 0.87%) |
-0.01s (- 0.27%) |
2.90s |
3.03s |
| Total Time |
9.49s (± 0.46%) |
9.45s (± 0.23%) |
-0.05s (- 0.48%) |
9.40s |
9.49s |
| TFS - node (v12.1.0, x64) |
|---|
| Memory used |
306,390k (± 0.03%) |
306,393k (± 0.01%) |
+3k (+ 0.00%) |
306,322k |
306,490k |
| Parse Time |
0.94s (± 0.71%) |
0.93s (± 0.43%) |
-0.01s (- 0.64%) |
0.92s |
0.94s |
| Bind Time |
0.71s (± 0.71%) |
0.70s (± 0.68%) |
-0.00s (- 0.28%) |
0.69s |
0.71s |
| Check Time |
4.18s (± 0.37%) |
4.17s (± 0.49%) |
-0.01s (- 0.22%) |
4.13s |
4.22s |
| Emit Time |
3.07s (± 0.75%) |
3.09s (± 0.87%) |
+0.02s (+ 0.78%) |
3.04s |
3.17s |
| Total Time |
8.88s (± 0.41%) |
8.90s (± 0.39%) |
+0.01s (+ 0.12%) |
8.81s |
8.97s |
| Angular - node (v8.9.0, x64) |
|---|
| Memory used |
351,846k (± 0.01%) |
353,479k (± 0.02%) |
+1,633k (+ 0.46%) |
353,290k |
353,547k |
| Parse Time |
2.10s (± 0.28%) |
2.12s (± 0.39%) |
+0.02s (+ 0.91%) |
2.09s |
2.13s |
| Bind Time |
0.93s (± 1.10%) |
0.94s (± 0.71%) |
+0.01s (+ 0.64%) |
0.92s |
0.95s |
| Check Time |
5.45s (± 0.65%) |
5.45s (± 0.77%) |
+0.00s (+ 0.07%) |
5.36s |
5.55s |
| Emit Time |
6.24s (± 0.96%) |
6.25s (± 1.30%) |
+0.01s (+ 0.13%) |
6.16s |
6.48s |
| Total Time |
14.71s (± 0.56%) |
14.75s (± 0.64%) |
+0.04s (+ 0.24%) |
14.60s |
15.05s |
| Monaco - node (v8.9.0, x64) |
|---|
| Memory used |
362,868k (± 0.01%) |
362,834k (± 0.01%) |
-34k (- 0.01%) |
362,783k |
362,893k |
| Parse Time |
1.56s (± 0.36%) |
1.56s (± 0.49%) |
-0.00s (- 0.19%) |
1.54s |
1.57s |
| Bind Time |
0.95s (± 0.74%) |
0.95s (± 0.52%) |
-0.00s (- 0.32%) |
0.94s |
0.96s |
| Check Time |
5.40s (± 1.36%) |
5.40s (± 1.86%) |
+0.01s (+ 0.13%) |
5.25s |
5.62s |
| Emit Time |
3.32s (± 3.05%) |
3.27s (± 4.67%) |
-0.05s (- 1.54%) |
2.96s |
3.46s |
| Total Time |
11.22s (± 0.41%) |
11.18s (± 0.57%) |
-0.05s (- 0.41%) |
11.04s |
11.32s |
| TFS - node (v8.9.0, x64) |
|---|
| Memory used |
323,413k (± 0.01%) |
323,432k (± 0.01%) |
+20k (+ 0.01%) |
323,353k |
323,543k |
| Parse Time |
1.26s (± 0.46%) |
1.27s (± 0.61%) |
+0.01s (+ 0.48%) |
1.26s |
1.29s |
| Bind Time |
0.76s (± 0.45%) |
0.76s (± 0.59%) |
+0.00s (+ 0.13%) |
0.75s |
0.77s |
| Check Time |
4.82s (± 0.66%) |
4.82s (± 0.47%) |
+0.00s (+ 0.00%) |
4.77s |
4.86s |
| Emit Time |
3.19s (± 0.64%) |
3.19s (± 0.94%) |
+0.00s (+ 0.00%) |
3.11s |
3.27s |
| Total Time |
10.04s (± 0.46%) |
10.04s (± 0.36%) |
+0.01s (+ 0.07%) |
9.97s |
10.14s |
| Angular - node (v8.9.0, x86) |
|---|
| Memory used |
200,041k (± 0.03%) |
200,900k (± 0.02%) |
+859k (+ 0.43%) |
200,804k |
200,997k |
| Parse Time |
2.02s (± 0.47%) |
2.04s (± 0.41%) |
+0.02s (+ 0.84%) |
2.03s |
2.07s |
| Bind Time |
1.04s (± 0.77%) |
1.05s (± 0.67%) |
+0.01s (+ 0.86%) |
1.03s |
1.06s |
| Check Time |
4.96s (± 0.85%) |
4.92s (± 0.47%) |
-0.03s (- 0.67%) |
4.88s |
4.97s |
| Emit Time |
6.08s (± 1.90%) |
6.07s (± 1.74%) |
-0.01s (- 0.15%) |
5.93s |
6.32s |
| Total Time |
14.11s (± 0.85%) |
14.09s (± 0.69%) |
-0.02s (- 0.14%) |
13.95s |
14.30s |
| Monaco - node (v8.9.0, x86) |
|---|
| Memory used |
203,707k (± 0.02%) |
203,727k (± 0.03%) |
+20k (+ 0.01%) |
203,627k |
203,836k |
| Parse Time |
1.60s (± 0.53%) |
1.60s (± 0.56%) |
-0.00s (- 0.25%) |
1.58s |
1.62s |
| Bind Time |
0.77s (± 0.86%) |
0.77s (± 0.64%) |
-0.00s (- 0.52%) |
0.76s |
0.78s |
| Check Time |
5.20s (± 1.80%) |
5.10s (± 0.76%) |
-0.10s (- 1.87%) |
5.05s |
5.23s |
| Emit Time |
3.13s (± 2.71%) |
3.20s (± 0.59%) |
+0.07s (+ 2.20%) |
3.17s |
3.27s |
| Total Time |
10.70s (± 0.50%) |
10.66s (± 0.43%) |
-0.03s (- 0.30%) |
10.58s |
10.80s |
| TFS - node (v8.9.0, x86) |
|---|
| Memory used |
182,586k (± 0.01%) |
182,558k (± 0.02%) |
-28k (- 0.02%) |
182,467k |
182,655k |
| Parse Time |
1.30s (± 0.76%) |
1.30s (± 0.82%) |
-0.01s (- 0.54%) |
1.27s |
1.33s |
| Bind Time |
0.71s (± 1.19%) |
0.71s (± 0.94%) |
+0.00s (+ 0.14%) |
0.70s |
0.73s |
| Check Time |
4.58s (± 1.09%) |
4.57s (± 0.48%) |
-0.01s (- 0.33%) |
4.52s |
4.62s |
| Emit Time |
2.95s (± 0.74%) |
2.96s (± 0.77%) |
+0.01s (+ 0.20%) |
2.92s |
3.02s |
| Total Time |
9.55s (± 0.72%) |
9.53s (± 0.34%) |
-0.02s (- 0.18%) |
9.45s |
9.59s |
System
| Machine Name | ts-ci-ubuntu |
|---|
| Platform | linux 4.4.0-166-generic |
|---|
| Architecture | x64 |
|---|
| Available Memory | 16 GB |
|---|
| Available Memory | 4 GB |
|---|
| CPUs | 4 × 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 |
36622 |
10 |
| Baseline |
master |
10 |
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Anders Hejlsberg (@ahejlsberg) How do these predicates relate to couldContainTypeVariables? Is there a reason these are ObjectFlags and that uses a property?
Context: instantiateType is frequently called on types that can't be instantiated and it seems like we could save some time (~2% on the material-ui benchmark above, in my naive implementation) by checking one or more of these cached values.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
How do these predicates relate to couldContainTypeVariables? Is there a reason these are ObjectFlags and that uses a property?
The isGenericObjectType and isGenericIndexType functions are used to determine whether conditional types, indexed access types, and index types should be resolved, or whether resolution should be deferred in a higher-order type of the particular kind. It is possible for these functions to return false even if the given type references type variables, e.g. for the type { foo: T } where T is a type variable.
The couldContainTypeVariables function computes whether a type possibly contains type variables, i.e. whether it possibly could be affected by instantiation. This function may return true even when the type references no type variables because it isn't possible to explore all types fully in depth. There's no good reason for this function to use a dedicated property. An ObjectFlags flag would be better.
In scenarios that are heavy on union and intersection type instantiation, it might make sense to first call couldContainTypeVariables since the cached result would allow us to bail out quicker.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Follow-up questions:
- Why do we only set those flags on union and intersection types? It looks like we might do non-trivial work for other types?
- Do we need both sets of flags? I would guess that only one of the two makes sense for any given type or that, if both make sense, only the
|| of the values matters.
Edit - offline responses:
- Claim is that it's only expensive for union and intersection types.
- Yes, the difference matters.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|