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

[data grid] Potential Memory Leak in the DataGrid component #15459

Closed
marcusguay opened this issue Nov 18, 2024 · 7 comments
Closed

[data grid] Potential Memory Leak in the DataGrid component #15459

marcusguay opened this issue Nov 18, 2024 · 7 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! performance status: waiting for author Issue with insufficient information

Comments

@marcusguay
Copy link

marcusguay commented Nov 18, 2024

Steps to reproduce

Steps:

  1. Open this link to live example: https://codesandbox.io/p/devbox/vite-react-forked-tgv2tj?workspaceId=ab98d85e-d21a-46f5-ab70-5be2562476c1
  2. Navigate to the collections page
  3. Navigate back
  4. Take heap snapshots
  5. Repeat this a bunch of times

Current behavior

In the Memory tab in the Chrome dev tools, taking snapshots (Both heap shots or detached nodes) after navigating to and back from both pages, detached nodes start accumulating.

Expected behavior

Nodes should not become detached when navigating to and away from the page and should eventually be cleared from memory.

Context

I am trying to use the component to display data, however, the detached nodes start to accumulate without being collected by the garbage collector.

Your environment

npx @mui/envinfo
System:
    OS: Linux 6.1 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    npm: 9.8.1 - /usr/local/bin/npm
    pnpm: 8.10.2 - /usr/local/share/npm-global/bin/pnpm
  Browsers:
    Chrome: Not Found
  npmPackages:
    @emotion/react: ^11.13.3 => 11.13.3 
    @emotion/styled: ^11.13.0 => 11.13.0 
    @mui/core-downloads-tracker:  6.1.7 
    @mui/material: ^6.1.7 => 6.1.7 
    @mui/private-theming:  6.1.7 
    @mui/styled-engine:  6.1.7 
    @mui/styled-engine-sc: ^6.1.7 => 6.1.7 
    @mui/system:  6.1.7 
    @mui/types:  7.2.19 
    @mui/utils:  6.1.7 
    @mui/x-data-grid: ^7.22.2 => 7.22.2 
    @mui/x-internals:  7.21.0 
    @types/react:  18.3.12 
    react: ^18.3.1 => 18.3.1 
    react-dom: ^18.3.1 => 18.3.1 
    styled-components: ^6.1.13 => 6.1.13 
</details>

Search keywords: memory leak Datagrid

@marcusguay marcusguay added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 18, 2024
@michelengelen michelengelen changed the title [DataGrid] Potential Memory Leak in the DataGrid component [data grid] Potential Memory Leak in the DataGrid component Nov 18, 2024
@michelengelen
Copy link
Member

@romgrk could you have a look at this one, please? 🙏🏼

@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Nov 18, 2024
@michelengelen michelengelen added the component: data grid This is the name of the generic UI component, not the React module! label Nov 18, 2024
@polyamakov
Copy link

Any news on this? We are worried about this too.

@cherniavskii
Copy link
Member

cherniavskii commented Nov 25, 2024

I don't think the issue with detached nodes is a Data Grid issue, it seems to be a React issue – see facebook/react#18116 and facebook/react#26069.
I've forked your demo and replaced Data Grid with 500 div elements: https://codesandbox.io/p/devbox/vite-react-forked-yx7d56?file=%2Fsrc%2Fpages%2Ftest.jsx%3A10%2C11

Screen.Recording.2024-11-25.at.19.44.00.mov

Do you experience specific performance issues?

@cherniavskii cherniavskii added performance status: waiting for author Issue with insufficient information and removed bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 25, 2024
@lauri865
Copy link
Contributor

lauri865 commented Nov 25, 2024

That seems to be the case, mostly at least. And explains why it's often the whole route's content that's being retained.

Interestingly, I also tried to forcefully remove the rootElement's subtree and rootElement itself on unmount, which reduced the memory allocation for the reproduction at least. But for some odd reason, Datagrid's main container still was retained, which could signal that there's a potential issue somewhere within Datagrid still 🤔.

On thing I noticed quickly is that the following timer is not cleared on unmount (but probably not the root cause of this issue):

const debouncedSetSavedSize = React.useMemo(
() => throttle(setSavedSize, props.resizeThrottleMs),
[props.resizeThrottleMs],
);

Reproduction: https://codesandbox.io/p/devbox/vite-react-forked-7frhqz?workspaceId=8a8eeabe-fead-479a-a993-25a9868e8015

Screenshot 2024-11-26 at 00 51 40

@lauri865
Copy link
Contributor

lauri865 commented Nov 26, 2024

To test, wait a few seconds before creating the profile, to avoid triggering it too early and retaining a reference before the garbage collector kicks in.

It doesn't get stuck on every navigation, but only on some it seems.

It doesn't seem to ever retain the grid footer, only the main container, and Mui-Box's node count is correctly reduced. Both of which also indicate that it's likely not connected to the React's issue.

@lauri865
Copy link
Contributor

But it could also be Emotion's memory leak in dev, which may not exist in prod. Hence, I'd suggest not to go down the rabbit hole to debug it, unless someone can demonstrably show a memory leak in production.

Copy link

github-actions bot commented Dec 3, 2024

The issue has been inactive for 7 days and has been automatically closed.

@github-actions github-actions bot closed this as completed Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! performance status: waiting for author Issue with insufficient information
Projects
None yet
Development

No branches or pull requests

5 participants