Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] Default theme props degrade performance #10033

Open
2 tasks done
LinnaViljami opened this issue Aug 14, 2023 · 9 comments · Fixed by mui/material-ui#43120
Open
2 tasks done

[DataGrid] Default theme props degrade performance #10033

LinnaViljami opened this issue Aug 14, 2023 · 9 comments · Fixed by mui/material-ui#43120
Assignees
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! performance

Comments

@LinnaViljami
Copy link

LinnaViljami commented Aug 14, 2023

Duplicates

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example:

https://codesandbox.io/s/react-typescript-forked-xddwq4?file=/src/App.tsx

Steps:

  1. Click row
  2. Check react profiler. Every row is rendered causing lagging performance.

Current behavior 😯

On click the whole grid is rerendered, including every row and header. This causes significant delay when rendering for example up to 20 rows with 10 columns = 200 cells

Expected behavior 🤔

Only rows which selection changes should be rerendered. No headers should be rerendered

Context 🔦

Row selection performance is not good enough

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID or Support key 💳 (optional)

72822

@LinnaViljami LinnaViljami added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 14, 2023
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Aug 14, 2023
@romgrk
Copy link
Contributor

romgrk commented Aug 14, 2023

You have clicked the "I have tested with the latest version" checkbox but it seems that you're not. Can you test with the latest version and confirm? This should be fixed.

@romgrk romgrk added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 14, 2023
@LinnaViljami
Copy link
Author

LinnaViljami commented Aug 15, 2023

I updated the latest packages now.

With react profilier we can see, that row rerendering is partially memorized. Headers are still all rerendered causing problems with large amounts. (for example 6ms per header causing significant lag if more than 10 headers)

I tested it out with 11 headers and 18 rows. The render time of the headers was 56,7ms, which happens on every row click. The render time of the rows was 24,5ms.

Here is the simplified profiler result with 2 columns and 8 rows

rows_performance

headers_performance

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Aug 15, 2023
@kealjones-wk
Copy link
Contributor

I just ran into this issue, with a bit of debugging I realized that this (at least for me) is caused by the presence of a custom theme specifying a defaultProps for MuiDataGrid.

Replicated here: https://codesandbox.io/s/focused-bas-rjzts7?file=/src/index.tsx:460-488

@romgrk
Copy link
Contributor

romgrk commented Jul 25, 2024

For some reason that sandbox is stuck in loading state for me, so I can't inspect that code.

Any prop that isn't a primitive or memoized can cause re-renders, e.g. the initialState inline object changes on each render so it can cause some re-rendering:

<DataGrid initialState={{ a: 1 }} />

More details here: https://mui.com/x/react-data-grid/performance/

@kealjones-wk
Copy link
Contributor

kealjones-wk commented Jul 26, 2024

@romgrk Idk why that code sandbox is broke sorry about that, anyway i re-created it here: https://stackblitz.com/edit/react-mxv54z?file=index.tsx This and the previous code sandbox are forks of that exact same link you gave: https://mui.com/x/react-data-grid/performance/#visualization The only difference is the customTheme and ThemeProvider in index.tsx.

Copy link

github-actions bot commented Aug 9, 2024

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@LinnaViljami: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

@romgrk romgrk reopened this Aug 9, 2024
@romgrk
Copy link
Contributor

romgrk commented Aug 9, 2024

This isn't completed yet, I need to refactor useThemeProps out for multiple reasons (this one and the design-system agnostic objective), I need to do more exploration.

@romgrk
Copy link
Contributor

romgrk commented Aug 9, 2024

I could do a temporary fix for a bit more work, is this very impactful for anyone?

@romgrk romgrk changed the title DataGridPro - rows and headers rendered on every click [DataGridPro] Rows and headers rendered on every click Aug 9, 2024
@romgrk romgrk changed the title [DataGridPro] Rows and headers rendered on every click [DataGrid] Rows and headers rendered on every click Aug 9, 2024
@romgrk romgrk changed the title [DataGrid] Rows and headers rendered on every click [DataGrid] Default theme props degrade performance Oct 1, 2024
@romgrk romgrk added the bug 🐛 Something doesn't work label Oct 1, 2024
@romgrk
Copy link
Contributor

romgrk commented Oct 1, 2024

@brijeshb42 Follow-up of the useThemeProps discussion: I've been trying to patch the issue here but it doesn't seem to be possible. We would need to re-implement useThemeProps to add a useMemo to it, but we don't have access to the default theme so we can't. What do you think about adding a stable option to that function to let it use useMemo only when required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! performance
Projects
Status: 🔖 Ready
Development

Successfully merging a pull request may close this issue.

4 participants