Improve performance for large unions with few non-primitives

[ypresto]

Suggestion

🔍 Search Terms

performance
emotion
material-ui
getKeyPropertyName
chooseOverload
instantiation
instantiate
large union

✅ Viability Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

TL:DR: Determine fast path in getKeyPropertyName() by the count of non-primitives in unions, not just the length of unions.

I'm investigating type check / vscode completion performance issue with material-ui v5, and found this one of root causes.
(See mui/material-ui#19113 (comment) for more detail of the investigation.)

getKeyPropertyName() is (indirectly) called from chooseOverload() to determine argument type matches to the method signature. It has fast path (that returns undefined) for small union with less than 10 constituents.

Currently it checks only one of: "length of union" OR "existence of non-primitive":

if (types.length < 10 || getObjectFlags(unionType) & ObjectFlags.PrimitiveUnion) {

This causes eager instantiation of large union type with many primitives + few non-primitives. For example it affects material-ui v5 (mui/material-ui#19113 (comment)) and emotion (emotion-js/emotion#2257).

While I'm not tested with emotion itself, both are using almost the same type. Profiler result of getDiagnostics() when editing the file contains styled() call of material-ui v5 (with some optimization) will looks like this:

This is because they have 10-12 constitutions in the union.

export type InterpolationPrimitive = // length: 10
  | null
  | undefined
  | boolean // true | false
  | number
  | string
  | ComponentSelector
  | Keyframes
  | SerializedStyles
  | CSSObject;

export type Interpolation<Props> = // length: 12
  | InterpolationPrimitive
  | ArrayInterpolation<Props>
  | FunctionInterpolation<Props>;

https://github.com/emotion-js/emotion/blob/f3c51e8aca287170e43110bee2e4527eacfcada4/packages/serialize/types/index.d.ts#L20-L29

Removing 3 items from InterpolationPrimitive will remove getKeyPropertyName() entry from above profiler result and improves performance (In above environment 170-210ms to <= 20ms).

But it'll break library feature.

So, in getKeyPropertyName(), count non-primitive constitutions instead of length can improve performance for primitives-rich unions.

📃 Motivating Example

Improves type check performance when you have large union with many primitives.

💻 Use Cases

See above.

[ypresto]

It is early but I wrote implementation of this (not sending PR until Backlog milestone):
main...ypresto:perf-large-primitive-union

[RyanCavanaugh]

Would be very interested to run the perf suites on this. Thanks for investigating!

[ypresto]

There seems no change on perf suites, it may be nice to have an editing scenario performance test..!
(I could not find perf suites source code on GitHub. Is it a private repo?)

For later reference, this is rough benchmark result for adding/removing extra line break in this file with vscode on my machine:

import { Button, styled } from '../../material-ui/src';

const foo = styled(Button)({ width: 100 });

perf-profile-mui-styled-button-get-key-property-name-patch.zip

getEncodedSemanticClassifications (Semantic Highlighting): 319.2ms -> 185.3ms
getSemanticDiagnostics: 237.0ms -> 130.8ms (without other optimization)