Unbearably slow typescript definitions

[charrondev]

Current behavior:

Some of the built-in types cause type-checking and VSCode intellisense feature to be incredibly, painfully slow.

To reproduce:

This only seems to affect large projects. For me that's working on https://github.com/vanilla/vanilla (working specifically with react components in library/src/scripts, but it reproduces in other areas as well).

I've recorded some gifs of the performance degradation.

Situation	Gif
Current Behaviour
With a slight modification of the type

Expected behavior:

Intellisense on a machine as powerful as the one I'm using should be instant. (32Gb of RAM, i9 CPU, lightning fast SSD).

Environment information:

• react version: 16.9.0
• @emotion/css version: 11.1.3

The slowness cause

Typescript has had a lot of issues recently with various libraries replicating CSS or react types. https://github.com/microsoft/TypeScript/issues/34801#issuecomment-767026973

In this case the cause of the slowness in particular seems to come from this piece of code

https://github.com/emotion-js/emotion/blob/master/packages/serialize/types/index.d.ts#L10-L14

export type CSSPropertiesWithMultiValues = {
  [K in keyof CSSProperties]:
    | CSSProperties[K]
    | Array<Extract<CSSProperties[K], string>> // The Extract<> here.
}

It seems the Extract<> on something with as many properties as this CSSProperties is very slow. What I've done for my own personal use was to change the code to

export type CSSPropertiesWithMultiValues = {
  [K in keyof CSSProperties]:
    | CSSProperties[K]
    | Array<CSSProperties[K]>>
}

Now this is may not be the correct type of what's allowed, as it it then allows multiple values an array of values to be passed to properties that don't support it. My question is until typescript improves the performance of Extract<> in a situation like this, what do we do?

[charrondev]

Of course immediately after filing the issue, I think I found a more correct solution.

export type CSSPropertiesWithMultiValues = {
  [K in keyof CSSProperties]:
    CSSProperties[K] extends string ? CSSProperties[K] | Array<CSSProperties[K]> : CSSProperties[K]
}

This seems to be the intention and doesn't cause the speed issues. I guess it's the Array<never> from Extract that seems to be causing the slowdown.

[charrondev]

I've opened a PR to patch the issue https://github.com/emotion-js/emotion/pull/2258

[jordanjlatimer]

Currently experiencing this as well. Hopefully the PR works.

[amcasey]

On my box, check time for vanilla is ~22 seconds. If I rewrite ArrayCSSInterpolation, it drops to ~18 seconds.

-export interface ArrayCSSInterpolation extends Array<CSSInterpolation> {}
+export type ArrayCSSInterpolation = Array<CSSInterpolation>;

Edit: updated with numbers for a warm disk. Less impressive, but still pretty good for a one line change.

[amcasey]

@charrondev I'd be interested to know why you think Array<never> is involved. I would have guessed it was distribution over the conditional.

[Andarist]

@charrondev I'd be interested to know why you think Array is involved. I would have guessed it was distribution over the conditional.

I have a vague memory of smth like never[] causing heavy deopts that you were surprised by and you were investigating the issue. I might not remember the issue correctly though.

I would have guessed it was distribution over the conditional.

The distribution itself should be a fairly easy one here - I suspect that there are just a few items to distribute for each property. Question would be - if we have a mapped type that distributes in each of its properties then are those distributes lazily or eagerly? The object that is being mapped is a pretty big one so if the whole thing would be evaluated eagerly (OTOH I would expect the result to be cached which should make the additional autocompletes fast) then maybe this time adds up?

It would be great if there would be some kind of documentation about what happens eagerly and what happens lazily - I understand that this is often a moving target for you guys but this would help tremendously library authors.

On my box, check time for vanilla is ~22 seconds. If I rewrite ArrayCSSInterpolation, it drops to ~18 seconds.

Huh, this is a weird one - your usual recommendation is to prefer interfaces over types. What makes this case different?

[amcasey]

I have a vague memory of smth like never[] causing heavy deopts that you were surprised by and you were investigating the issue. I might not remember the issue correctly though.

Hopefully, you mean TS and not me personally, because I don't remember that investigation. 😅 Having said that, it's not obvious to me how that could cause a deopt because the code that's executing is the compiler/LS, which is not seeing never[], but rather a type representing it.

The distribution itself should be a fairly easy one here - I suspect that there are just a few items to distribute for each property.

If it adds up to hundreds of (small) subtype reductions, it can still cause a problem. That was the expensive part of #42824, for example.

Question would be - if we have a mapped type that distributes in each of its properties then are those distributes lazily or eagerly?

My guess would be lazily, but "it depends" is probably more accurate. It's a characteristic of the compiler that we're still fine-tuning (e.g. I believe things got lazier to allow recursive conditional types).

Huh, this is a weird one - your usual recommendation is to prefer interfaces over types.

I think you're referring to this pointer. I believe it was intended to refer to intersections with more than one member.

What makes this case different?

https://github.com/microsoft/TypeScript/issues/42995#issuecomment-787089468

[amcasey]

It's not very exciting, but I saved another half-second or so by pulling out an explicitly-typed constant for https://github.com/vanilla/vanilla/blob/7cc3ce7c31e181d934514b78c6f8406f7b5c5e3a/library/src/scripts/content/userContentStyles.ts#L720. (Otherwise, it gets checked repeatedly as part of overload resolution. Not something you should think about, in general, just a hotspot in this particular code.)

[Andarist]

Hopefully, you mean TS and not me personally, because I don't remember that investigation. 😅 Having said that, it's not obvious to me how that could cause a deopt because the code that's executing is the compiler/LS, which is not seeing never[], but rather a type representing it.

I've tried to find the issue I had a memory of... but it seems that it was just a dream... or something :P Can't find it so it probably didn't happen.

If it adds up to hundreds of (small) subtype reductions, it can still cause a problem. That was the expensive part of #42824, for example.

Wow, this is... something. Quite impressed that the OP has figured out the workaround on their own.

I think you're referring to this pointer. I believe it was intended to refer to intersections with more than one member.

Yep, was referring to this.

microsoft/TypeScript#42995 (comment)

Thanks for the pointer! It makes sense - although it's quite a hard rule to remember (it's not super intuitive).

It's not very exciting, but I saved another half-second or so by pulling out an explicitly-typed constant for https://github.com/vanilla/vanilla/blob/7cc3ce7c31e181d934514b78c6f8406f7b5c5e3a/library/src/scripts/content/userContentStyles.ts#L720. (Otherwise, it gets checked repeatedly as part of overload resolution. Not something you should think about, in general, just a hotspot in this particular code.)

What do you mean by the overload resolution here? None of the called function and the outer one has any overloads 🤔

[amcasey]

Thanks for the pointer! It makes sense - although it's quite a hard rule to remember (it's not super intuitive).

I'm still trying to figure out how best to document this. Frankly, it would never have occurred to me to subtype an array type without adding members, so it seems strange to recommend against it. I can add a footnote about array types to the intersection advice, if you think that would be helpful.

What do you mean by the overload resolution here? None of the called function and the outer one has any overloads 🤔

I was referring to the three lines above the "called function" you linked as overloads.

[charrondev]

I see you got a bit of reduction in the general type checking of the whole project, but I don't think I was negatively impacted by that time.

The negative impact is in VSCode with the language service slowing down significantly.

• Auto-imports go from taking ~300ms to 5+ seconds.
• Jump-to-definition goes from taking ~300ms to 5+ seconds.
• It can take 10+ seconds for the language service to recognize that the file was changed and clear out previous errors.

[charrondev]

Are there steps I can take to profile the language server itself? I tried logging traces in VSCode and I could definitely see the slow responses but wasn't able to really pinpoint any specific thing slowing it down. The PR I opened here was really was found by me going through commonly used types and swapping them out with any until things sped up.

If there are specific steps I can take to pinpoint them I'm open to trying them.

[amcasey]

@charrondev As nice as a reduced repro would be, in this case, a full repro would be fine. If you point me at a commit (in Vanilla, I assume) and give me a list of editing steps, I'll see what I can do.

Having said that, if you're on TS 4.2, you should also be able to use VS Code's typescript.tsserver.EnableTracing to generate a trace similar to the one produced by tsc --generateTrace.

[charrondev]

Alright. I'll give it a check. We work on a private fork of vanilla/vanilla day-to-day but I'll make sure I can get a reproduction with the open source repo. Currently the project itself is on TS 4.0.x but I see the same language server slowdowns with the nightly typescript in VSCode as well.

[amcasey]

Tweaks to get it building with TS 4.2: https://github.com/amcasey/vanilla/tree/ts42

The changes are all correct except that I didn't know how to fix the usages of global name in MentionSuggestion.tsx, so I just changed them to "HACK".

[charrondev]

Sorry for the delay. I should have a reproduction ready on TS 4.2 officially by next week. I've been a bit swamped in my day job recently.

[amcasey]

I expect big perf wins for Emotion from these changes in TS 4.3: https://github.com/microsoft/TypeScript/pull/43623 https://github.com/microsoft/TypeScript/pull/43624

[Andarist]

A workaround for the second one is already prepared on our side but I did not have time to get it through the finish line just yet: https://github.com/emotion-js/emotion/pull/2296 .

Thanks for investigating the first case as well - I don't quite understand the fix in that PR. Why do you need to introduce any sets for those temporary relations? Couldn't the full cached interface be just computed there to reach the same behavior as in the "fast code"? I mean - both are semantically equivalent, only the order of two lines differ. So why the existing code path in TS can't be simply reused? 🤔

[amcasey]

@Andarist The PR replaces a linear lookup in an array with a fast lookup in a set. In emotion, the array is very large and the array lookups were taking a long time.

[Andarist]

I understand the nature of this low level optimization but i was rather curious as to why the source code order mattered so much here and why the codepaths could not be unified in a way that the "usage before definition" (the slow one) would just act in the very same way as "definition before usage" (the fast one). Or is the optimization for both cases?

[amcasey]

Oh, sorry, I thought you were asking about the order of the code in the compiler.

https://github.com/microsoft/TypeScript/issues/43437#issuecomment-816037694

[Andarist]

Ah yes - i've seen this comment already and i guess this part is especially relevant:

When the declarations in the repro example are reversed, a number of the interesting relationships are evaluated as part of checking the interface declaration and therefore cached in the main relationship cache. Thus, when it comes time to evaluate a relationship involving the interface, we already have most of the results cached and they never end up on the "maybe related" list.

I lack a lot of context here though and if the answer would be too in-depth then disregard me question. I was just curious why the source ordered has mattered since both codes were semantically equivalent (unless they were not and in such a case i would love to learn whats the difference). This describes why the fast input code gets fast typechecking - im wondering why the slow input code doesnt get typecked through the very same code path in TS codebase (which would result in equally fast typechecking, from what i understand).

Anyhow - im just curious 😅

[amcasey]

It's largely over my head too, but I understand Anders' comment to say that sometimes type comparisons are done in isolation and sometimes they're done as part of other work. Which one happens for a given declaration depends on the first time it is needed, which depends on the order of the declarations (i.e. is deterministic but arbitrary). When they're done in isolation, we trust (i.e. cache) the results immediately. When they are intermediate results, we wait until the "stack" of checks is unwound before trusting (i.e. caching) the results. Unfortunately, sometimes other intermediate results in the same evaluation "stack" will depend on the not-yet-cached intermediate result, causing the work to be redone.

Why don't we topologically sort them? I wasn't there but I would guess it was either too hard in the general case or didn't seem like it would make a difference (this is a pathological case).

Can this be generalized into guidance for type authors? We hope not - there wasn't expected to be a perf difference and these PRs are expected to eliminate the perf difference we've discovered. In particular, while the bug seems to imply that you want to declare interfaces before consts, that is (as far as we know) a fluke, specific to this example.

For future readers, DO NOT REORDER YOUR CODE HOPING TO IMPROVE PERF. If you do it anyway and find that it helps, please file a bug against TypeScript.

[Andarist]

DO NOT REORDER YOUR CODE HOPING TO IMPROVE PERF.

And you are telling me this after I've implemented an eslint rule for this? 😬

On a more serious note though - thank you for explaining this in more detail. I really appreciate it and it makes more sense now to me (even though I would intuitively think that this is resolved in topological order).

[amcasey]

I can't speak for this compiler, but the C# compiler assumed things could be evaluated in any order because it also supported the language service, where the user decides what's interesting.

[mattrunyon]

Any updates on this? This could be adding to slow behavior in dependent packages like MUI

https://github.com/mui-org/material-ui/issues/19113

[Andarist]

IIRC the problems that were recognized in this thread were, sort of, addressed in TS itself so it should work better with the latest version of TS. If there is anything that we can do to improve the situation - let us know. I'm afraid that we don't have time resources to investigate what exactly causes a slowdown though.