microsoft / microsoft/TypeScript
Possible performance improvement around types with `this` argument
Nessuno ha ancora preso questa issue.
- Lingua principale
- Go
- Stelle
- 111k
- Fork
- 14.4k
- Merge medio
- 1g 19h
- PR unite (30g)
- 117
Descrizione
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:
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:
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:
- You remove
ClientBabelPresetOptions - You add a second generic parameter,
PluginObj
To reproduce this in graphql-code-generator:
- Clone
- Install deps:
yarn - Run TSC
🙁 Actual behavior
Comparing two similar types takes a long time.
🙂 Expected behavior
Skip getVariancesWorker being called in this case.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia con la riproduzione collegata in graphql-code-generator packages/presets/client/src/babel.ts e nel TypeScript playground, quindi traccia checkTypeArguments, fillMissingTypeArguments, getTypeWithThisArgument e getVariancesWorker. Confronta le identità dei tipi e il lavoro sulla varianza quando gli argomenti generici vengono forniti da una sola dichiarazione rispetto a quando vengono forniti da entrambe. Il lavoro è completato quando viene confermato un modo sicuro per evitare il confronto costoso non necessario, con un test di regressione che copra il pattern segnalato.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- typescript
- Ambito
- compilers, performance
- Tipo di issue
- Bug
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Stato di attività
- Ferma
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 32/100