Skip to content
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

Cache results of isGenericObjectType and isGenericIndexType #36622

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Feb 5, 2020

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%.

@ahejlsberg
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 5, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at a0546fc. You can monitor the build here. It should now contribute to this PR's status checks.

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 - 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 Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory4 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)
  • 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

@ahejlsberg ahejlsberg merged commit b8b5948 into master Feb 6, 2020
@amcasey
Copy link
Member

amcasey commented Feb 21, 2020

@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.

@ahejlsberg
Copy link
Member Author

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.

@amcasey
Copy link
Member

amcasey commented Feb 21, 2020

Follow-up questions:

  1. Why do we only set those flags on union and intersection types? It looks like we might do non-trivial work for other types?
  2. 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:

  1. Claim is that it's only expensive for union and intersection types.
  2. Yes, the difference matters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants