angular / material

Material design for AngularJS
https://material.angularjs.org/
MIT License
16.54k stars 3.39k forks source link

dialog: generating detached DOM nodes when closed #11207

Open vinaynb opened 6 years ago

vinaynb commented 6 years ago

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Using $mdDialog with custom template should not result in detached DOM nodes.

What is the current behavior?

When using $mdDialog and after closing it via $mdDialog.hide(), the heap snapshots show lot of detached DOM nodes which point to instanceCache in $mdDialog service. Check out the heap snapshot screenshot

screenshot from 2018-04-04 16 06 41

I tried to figure out the internals and i think mdDialog is setting up some kind of internal cache which re-uses things when same modal's are opened multiple times but i failed to identify it concretely and clear those detached nodes from memory. I target to run my application on raspberry pi and it has quite a few modals hence increasing detached dom node count is warning signal for me leading to memory exhaustion over time. Hence i guess i need someone to familiar with internals to point me in right direction and if at all there is something leaking with mdDialog ?

CodePen and steps to reproduce the issue:

CodePen Demo which shows your issue:

I will try to reproduce this issue on codepen but as of now i don't have a pen.

Detailed Reproduction Steps:

I am using ui router and while switching between states if i open a modal with custom template and close it and then check the snapshot it shows detached nodes. Those nodes don't show if i don't open modal at all.

Which versions of AngularJS, Material, OS, and browsers are affected?

Angular js - 1.6.8 OS - linux/ubuntu 14.04 browser - chrome 65

Splaktar commented 6 years ago

What version of AngularJS Material is in use?

Please provide a demo and reproduction steps for checking the heap and observing detached nodes. Also, are the nodes eventually garbage collected over time or do they keep accumulating?

vinaynb commented 6 years ago

Angular material version - 1.1.9 (forked) I have created a basic repro here.

I must note that although there are detached nodes in the pen i mentioned above, the retainer graph is for them is not the same as i have in my angular application. But maybe this example might help in debugging.

Steps to reproduce

Also, are the nodes even garbage collected over time or do they keep accumulating?

From what i see they are not being gc'ed even though i fired it manually but at the same time when they do not keep accumulating if it open the same dialog multiple times. Maybe its caching at play here.

screenshot from 2018-04-06 12 28 23 screenshot from 2018-04-06 12 29 03

Splaktar commented 6 years ago

https://github.com/angular/material/issues/10851 also mentions that $mdDialog is leaking memory.

Splaktar commented 6 years ago

https://github.com/angular/material/issues/11493 has some additional debugging information that is related to this issue.

Splaktar commented 5 years ago

Here's a link to the original CodePen built on 1.1.8. Here's that same CodePen upgraded to 1.1.20.

With 1.1.8 and 1.1.20, I see the new DialogController instance created on the Scope for each time the dialog is opened and closed, but the previous instance's Scope has been removed.

Splaktar commented 5 years ago

As for the "Detached DOM tree" mentioned above, I'm still working on finding this as it appears the Chrome DevTools have renamed these and made the Detached nodes more granular. It looks like they may be listed under "Detached HTMLDivElement" now.

Splaktar commented 5 years ago

If I look at "Detached HTMLDivElement" in 1.1.20, I see the following trend with heap snapshots after each open/close of the dialog:

Heap snapshot size starts at 5.9MB and increases to 9.5MB where it stabilizes between 9.5-9.6MB.

With 1.1.8, I see the following trend

Heap snapshot size starts at 10.1MB and increases to 9.5MB where it stabilizes between 9.4-9.6MB.

So there does appear to still be an issue with leaking detached DOM nodes related to opening and closing of $mdDialogs. However, it's not clear that this is actually leading to a significant JS Heap memory leak as the JS associated with these nodes does appear to be getting GC'd.

Splaktar commented 5 years ago

I did a profiling run where I opened and closed the dialog every few seconds. This is the result Screen Shot 2019-09-09 at 13 47 48

You can see that the JS Heap isn't leaking, but that we keep generating more and more DOM Nodes and listeners each time the dialog is opened and closed. We start at ~2,200 DOM nodes and ~60 listeners and end up with ~9,000 DOM nodes and ~90 listeners after just 30s.

Splaktar commented 4 years ago

In https://github.com/angular/angular.js/issues/4864#issuecomment-29394307, it is mentioned that ngAnimate causes this same behavior of Nodes and listeners increasing over time, but the JS Head remaining stable. We of course do use ngAnimate.

Splaktar commented 4 years ago

Screen Shot 2020-09-13 at 02 40 46

Here's a profiling session with 1.2.0 that was about 50 seconds long. You can see that, like the JS Heap, the Nodes and listeners are garbage collected away (less often than for the JS).

I've found a minor issue where we may leak two listeners per dialog closing, but fixing it doesn't really change the behavior in these profiling sessions.

Splaktar commented 4 years ago

I tried with ngAnimate disabled via $animate.enabled(false); but it doesn't seem to have a significant impact:

Screen Shot 2020-09-13 at 04 22 09

Splaktar commented 4 years ago

I'm going to lower the priority on this because I think that most of the leaks related to this have been resolved. The Nodes and Listeners tha increase, do seem to be garbage collected.

I would be happy to dig into this some more if someone could point out a specific use case/repro with AngularJS Material 1.2.0 (or hopefully 1.2.1 soon) along with some of their investigation findings.