microsoft / microsoft/TypeScript

Possible performance improvement around types with `this` argument

Open
#54,260 2 comments 13 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Domain: Performance Help Wanted Possible Improvement
Dominant language
Go
Stars
111k
Forks
14.3k
Avg merge
1d 19h
Merged PRs (30d)
117

Description

Bug Report

I've found a hotspot in a graphql-code-generator project where a comparison of types was taking a significant amount of time:

CleanShot 2023-05-16 at 11 15 26@2x

It comes from a packages/presets/client/src/babel.ts file, and it's related to the usage of a declare function. Specifically, the comparison between PluginObj types was costly.

The declare function from Babel, has the following signature:

export function declare<
    O extends Record<string, any>,
    R extends babel.PluginObj = babel.PluginObj
>(
    builder: (api: BabelAPI, options: O, dirname: string) => R,
): (api: object, options: O | null | undefined, dirname: string) => R;

I added the first generic parameter to the usage of declare, and it successfully eliminated the hotspot:

type ClientBabelPresetOptions = {
  artifactDirectory?: string;
  gqlTagName?: string;
};
export default declare<ClientBabelPresetOptions>((api, opts): PluginObj => {
// ...

Trace after that change:
CleanShot 2023-05-16 at 11 09 40@2x

I understand that by doing this, TS could use a default for the second parameter, R, and hence skip the inference.

But here comes an interesting find — when I provided both generic parameters, it went back to the same (slow) performance result as in the beginning:

export default declare<ClientBabelPresetOptions, PluginObj>((api, opts): PluginObj => {

Note: using PluginObj vs PluginObj<PluginPass> or even PluginObj<any> yields the same problem.

I paired with @Andarist, and he found out that even though the types looked the same, the target type (PluginObj) had an extra this argument. That led to getVariancesWorker being called on rather expensive types.

Now, the question is: why in checkTypeArguments the fillMissingTypeArguments doesn't use the this type anyhow but the constraint's type is obtained with the this type:

getTypeWithThisArgument(instantiateType(constraint, mapper), typeArgument)

If they would share the same ID then this particular case could work much faster.

🔎 Search Terms

performance, this, babel, getTypeWithThisArgument

🕗 Version & Regression Information
  • TypeScript version: 5.0.4
⏯ Playground Link

Playground link with relevant code

💻 Code

The code could be found here: https://github.com/dotansimha/graphql-code-generator/blob/86ec182887698742af8e9f47ffe39f07772e54a4/packages/presets/client/src/babel.ts#L16.

Currently, this file has an "improved" version, but upon playing with it you can notice long check times when:

  1. You remove ClientBabelPresetOptions
  2. You add a second generic parameter, PluginObj

To reproduce this in graphql-code-generator:

  1. Clone
  2. Install deps: yarn
  3. Run TSC
🙁 Actual behavior

Comparing two similar types takes a long time.

🙂 Expected behavior

Skip getVariancesWorker being called in this case.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the linked graphql-code-generator packages/presets/client/src/babel.ts reproduction and the TypeScript playground, then trace checkTypeArguments, fillMissingTypeArguments, getTypeWithThisArgument, and getVariancesWorker. Compare the type identities and variance work when one versus both declare generic arguments are supplied. Done means confirming a safe way to avoid the unnecessary expensive comparison, with a regression test covering the reported pattern.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.