material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Some bad ts performance cases

Open tjx666 opened this issue 1 year ago • 2 comments

Steps to reproduce

I write many details in the readme of https://github.com/tjx666/ts-perf-issue

Current behavior

The cases described in https://github.com/tjx666/ts-perf-issue/tree/main still bad ts performance

Expected behavior

Improve these cases performance.

Context

https://github.com/tjx666/ts-perf-issue

Your environment

npx @mui/envinfo
 System:
    OS: macOS 14.5
  Binaries:
    Node: 20.15.0 - ~/.local/state/fnm_multishells/22122_1719478157230/bin/node
    npm: 10.7.0 - ~/.local/state/fnm_multishells/22122_1719478157230/bin/npm
    pnpm: 9.4.0 - ~/.local/state/fnm_multishells/22122_1719478157230/bin/pnpm
  Browsers:
    Chrome: 126.0.6478.127
    Edge: Not Found
    Safari: 17.5
  npmPackages:
    @mui/lab: 5.0.0-alpha.170 => 5.0.0-alpha.170 
    @mui/material: ^5.15.20 => 5.15.20 
    @types/react: ^18.3.3 => 18.3.3 
    react: ^18.3.1 => 18.3.1 
    typescript: ^5.5.2 => 5.5.2

Search keywords: typescript, suggestion, slow, bad, performace, sxProps, createTheme, @mui/system

tjx666 avatar Jun 27 '24 09:06 tjx666

Thank you so much for the detail explanations!

siriwatknp avatar Aug 23 '24 03:08 siriwatknp

I just discovered that in my case, i had performance issues completely disappear when removing the @mui/icons-material package.

Then i tried downgrading to an older version and that seem to have improved my performance by a lot.

Version 5.11.11 seem to be much more stable in performance, but it requires the use of the --legacy-peer-deps flag. "@mui/icons-material": "^5.11.11"

I noted the issues happening here. (I have not tried other versions) "@mui/icons-material": "^6.1.4"

My Package.json
  
  "dependencies": {
    "@emotion/cache": "^11.13.1",
    "@emotion/react": "^11.13.3",
    "@emotion/styled": "^11.13.0",
    "@fontsource/roboto": "^5.1.0",
    "@mdx-js/loader": "^3.0.1",
    "@mdx-js/react": "^3.0.1",
    "@mui/icons-material": "^5.11.11",
    "@mui/material": "^6.1.4",
    "@mui/material-nextjs": "^6.1.4",
    "@next/mdx": "^14.2.15",
    "gray-matter": "^4.0.3",
    "mdast-util-to-string": "^4.0.0",
    "next": "14.2.15",
    "react": "^18",
    "react-dom": "^18",
    "rehype-autolink-headings": "^7.1.0",
    "rehype-slug": "^6.0.0",
    "remark": "^15.0.1",
    "remark-footnotes": "^5.0.0",
    "remark-frontmatter": "^5.0.0",
    "remark-gfm": "^4.0.0",
    "remark-mdx-images": "^3.0.0",
    "unist-util-visit": "^5.0.0"
  },
  "devDependencies": {
    "@types/mdx": "^2.0.13",
    "@types/node": "22.7.6",
    "@types/react": "^18",
    "@types/react-dom": "^18",
    "eslint": "^8",
    "eslint-config-next": "14.2.15",
    "typescript": "^5"
  }

OS Environment

npx @mui/envinfo
 System:
    OS: Windows 11 10.0.22631
  Binaries:
    Node: 20.15.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.7.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.9.0 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Chrome: Not Found
    Edge: Chromium (128.0.2739.54)
  npmPackages:
    @emotion/react: ^11.13.3 => 11.13.3
    @emotion/styled: ^11.13.0 => 11.13.0
    @mui/core-downloads-tracker:  6.1.4
    @mui/icons-material: ^5.11.11 => 5.16.7
    @mui/material: ^6.1.4 => 6.1.4
    @mui/material-nextjs: ^6.1.4 => 6.1.4
    @mui/private-theming:  6.1.4
    @mui/styled-engine:  6.1.4
    @mui/system:  6.1.4
    @mui/types:  7.2.18
    @mui/utils:  6.1.4
    @types/react: ^18 => 18.3.11
    react: ^18 => 18.3.1
    react-dom: ^18 => 18.3.1
    typescript: ^5 => 5.6.3

mikk809h avatar Oct 17 '24 07:10 mikk809h

I started seeing degraded code completion performance from @mui/icons-material: "6.1.0", downgrading to @mui/icons-material: "6.0.2" fixed the issue.

gkiely avatar Nov 11 '24 03:11 gkiely

We've been having problems with our application's createTheme calls (taking excessively long to compile, causing editor slowdowns in VS Code as it tries to parse the types, and - apparently - causing out-of-memory errors in Node.js in our CI/CD), so I investigated.

As I understand it, the problem is that the Components type is extremely complex: it references every component in MUI and (through augmentation) MUI-X, and the OverridesStyleRules and Interpolation types are themselves very complex. This results in a huge number of generic types that have to be instantiated.

I spent a long time exploring ways to improve that and came up with a couple of approaches.

Using as a baseline, tsc -p . --incremental false --diagnostics gives the following relevant numbers:

Instantiations:  744661
Memory used:    398768K
Total time:       3.77s

Option 1: Incremental improvements

overrides.ts references Interpolation<Props>, but it always passes a theme and always passes an ownerState of Record<string, unknown>. If I replace the general Interpolation<Props> with one that's customized to take advantage of this and name some complex types involved:

Instantiations:  700107
Memory used:    263230K
Total time:       2.32s

Option 2: Remove <Theme> from Components

The Components type takes a generic Theme parameter. Within the MUI codebase, it's instantiated 3 times:

Changing Components to be non-generic (for this experiment, replacing its Theme generic parameter with a hard-coded BaseTheme) is a huge help:

Instantiations:  296259
Memory used:    162207K
Total time:       0.93s

I believe there's room for cleanup here: e.g., if I understand correctly, createThemeWithVars versus createThemeNoVars (and, thus, ThemeOptions['components']' Components<Omit<Theme, 'components'>> versus [CssVarsThemeOptions['components']'s Components<Omit<Theme, 'components' | 'palette'> & CssVarsTheme>) is an implementation detail, so I'm not sure that it needs to be reflected in the types. Since theme options are optional anyway, could Components be changed to reflect the broadest subset of potential options? (I guess that's Components<Omit<Theme, 'components' | 'palette'> & CssVarsTheme>?)

Option 3: Change components to be a generic option

As a more drastic approach, change Components to be a generic record over whatever components are provided, rather than fully itemizing every component:

export type Components<Name extends keyof ComponentsPropsList, Theme = unknown> = Partial<Record<Name, {
  defaultProps?: ComponentsPropsList[Name];
  styleOverrides?: Name extends keyof ComponentNameToClassKey ? ComponentsOverrides<Theme>[Name] : never;
  variants?: ComponentsVariants<Theme>[Name];
}>>;

Change Theme.components to be opaque. Let ThemeOptions.components infer the supported props from whatever options are provided.

Instantiations:  296499
Memory used:    162306K
Total time:       0.89s

This would be backwards-incompatible and seems to give worse performance than the second option. (It's comparable for this test case but gets worse as more styles are overridden.)

If you'd like for me to open PRs with option 1 and/or 2 for further discussion, please let me know.

joshkel avatar Jun 04 '25 18:06 joshkel

@joshkel thanks for testing this out and proposing improvements.

@siriwatknp may I ask you to look at the proposals and evaluate if there's any we could implement?

DiegoAndai avatar Jun 06 '25 19:06 DiegoAndai