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

Non-persistent components aren't garbage collected

Open fonty422 opened this issue 1 year ago • 6 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Steps to reproduce 🕹

Link to live example: Example Steps:

  1. Copy the example for a basic menu and run locally in your own app
  2. Start the app and note the DOM Node count, manually garbage collect to confirm the count is correctly low
  3. Open and close the menu and note the DOM Node count increase
  4. Manually garbage collect and note they stay higher than before the menu was first opened
  5. Continue steps 3 and 4 and watch the DOM Node count continue every higher each time

Current behavior 😯

It seems like the components that don't persist (like menus and tool tips) that they are not garbage collected, or that I'm implementing them very wrong.

If it's a rarely clicked button, that's not so much of an issue, but we have a large virtualised list of Accordions with these components in the summary whose data is changing quite regularly. Scrolling through this list dramatically increases the DOM nodes and the tooltips become detached and sit in memory forever until a page refresh.

I can understand if it stays in memory because the button calling the menu is still there and it's not inefficient, but to keep creating new nodes every time while not clearing out the old ones appears wrong.

Is this a known issue, or have I just implemented it wrong?

Expected behavior 🤔

Dom Nodes should not continue to increase on multiple open/close events on the same component

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo
  System:
    OS: Windows 10 10.0.19045
  Binaries:
    Node: 19.4.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 9.2.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: master_preferences
    Edge: Spartan (44.19041.1266.0), Chromium (111.0.1661.62)
  npmPackages:
    @emotion/react: ^11.7.0 => 11.10.4
    @emotion/styled: ^11.6.0 => 11.10.4
    @mui/base:  5.0.0-alpha.97
    @mui/core-downloads-tracker:  5.10.5
    @mui/icons-material: ^5.2.0 => 5.10.3
    @mui/lab: ^5.0.0-alpha.60 => 5.0.0-alpha.99
    @mui/material: ^5.2.1 => 5.10.5
    @mui/private-theming:  5.10.3
    @mui/styled-engine:  5.10.5
    @mui/styled-engine-sc: ^5.1.0 => 5.10.3
    @mui/system:  5.10.5
    @mui/types:  7.2.0
    @mui/utils:  5.10.3
    @types/react:  18.0.20
    react: ^17.0.2 => 17.0.2
    react-dom: ^17.0.2 => 17.0.2
    styled-components: ^5.3.3 => 5.3.5

fonty422 avatar Apr 01 '23 21:04 fonty422

Just as an additional note, just opened another web app created by a different department but using the same tools, and found they had ~140k DOM Nodes which wouldn't clear with 800MB memory used for a super basic app. A refresh of the page showed only 515 nodes and 13MB memory used, so something is not quite right here. I can't imagine that we're all independently using it the wrong way.

fonty422 avatar Apr 01 '23 23:04 fonty422

I wonder whether it might be related to the issue with custom child elements? While the examples I gave do not use custom child elements (it's just a button wrapped in a tooltip), could the issue stem from the same thing where the ref isn't passed to the child correctly?

fonty422 avatar Apr 01 '23 23:04 fonty422

Are you using production version? What DOM nodes are being created? I am asking because we recently had similar issue, which was related to the dev mode usage, where emotion creates new style tag for the elements, which may be causing this false positive that there is a memory leak, see https://github.com/mui/material-ui/issues/36294. I will need more information on this.

mnajdova avatar Apr 06 '23 11:04 mnajdova

Hopefully I understand this correctly, but the test was done with a production version (i.e. a built app). But it's possible that the same thing is happening as in your linked issue. The problem is amplified in a non-production version. I'll be honest and say I'm finding it incredibly hard to actually find details on the nodes created. I did get a few heap snapshots to find the detached elements, but looking through those didn't seem to clearly identify where the issue was coming from. I'll make sure to have a look again and try get some further details for you.

fonty422 avatar Apr 10 '23 14:04 fonty422

@fonty422 I'm not able to reproduce the issue locally following the steps you provided i.e. the DOM node count doesn't increase every time the menu is opened. Can you provide a reproduction?

aarongarciah avatar May 14 '24 12:05 aarongarciah

Hey @aarongarciah, I left this project alone for some time and have just come back to it. Let me see if this is still a problem in production and if so I'll post, otherwise I'll close it.

fonty422 avatar May 15 '24 10:05 fonty422