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

Non-persistent components aren't garbage collected #36749

Open
2 tasks done
fonty422 opened this issue Apr 1, 2023 · 6 comments
Open
2 tasks done

Non-persistent components aren't garbage collected #36749

fonty422 opened this issue Apr 1, 2023 · 6 comments
Assignees
Labels
component: menu This is the name of the generic UI component, not the React module!

Comments

@fonty422
Copy link

fonty422 commented Apr 1, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • 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 fonty422 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 1, 2023
@fonty422
Copy link
Author

fonty422 commented Apr 1, 2023

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
Copy link
Author

fonty422 commented Apr 1, 2023

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?

@zannager zannager added the component: menu This is the name of the generic UI component, not the React module! label Apr 3, 2023
@mnajdova
Copy link
Member

mnajdova commented Apr 6, 2023

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 #36294. I will need more information on this.

@mnajdova mnajdova 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 Apr 6, 2023
@fonty422
Copy link
Author

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.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Apr 10, 2023
@aarongarciah
Copy link
Member

@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?

@fonty422
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

4 participants