Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Bug: Memory leak on removing app's root element #15239

Closed
djidel opened this issue Oct 10, 2016 · 4 comments
Closed

Bug: Memory leak on removing app's root element #15239

djidel opened this issue Oct 10, 2016 · 4 comments

Comments

@djidel
Copy link

djidel commented Oct 10, 2016

Probable Bug

Behavior: Memory leak after removing the app (by removing the app's root element)
Bug reproduction: Please check this demo http://codepen.io/djidel/pen/PGREXm
The n is how many times a primitive angular app is bootstrapped then removed.
With n=1 the whole page uses about 30Mb.
With n=500 the page uses about 70Mb.
Removing the app's root element seems to have no effect in memory use.

The expected behavior: Removing angular app's root element should "destroy" the app and clean up all the used memory.

Motivation / use case:
A web app that bootstraps angular apps on independent DOM objects while user interacts. Long use of the web app should not result in high memory use.

Angular version: 1.5.5
Browser: Google Chrome
Browser version: 53
OS: Windows

@gkalpak
Copy link
Member

gkalpak commented Oct 10, 2016

This seems to be somehow related to jQuery (and probably to its rewrapping behavior - or bad use of it).

I tried using jqLite and (after garbage collecting) the memory drops back to the same level as for n = 1.

With jQuery, the line that is responsible for the biggest percentage of the memory leak, is re-wrapping the $rootElement (which is unnecessary). By removing the unnecessary re-wrapping, the memory (after garbage collecting) drops back to ~2MB higher than the n = 1 case:

// By replacing:
var e = lastModule.get('$rootElement');
jQuery(e).remove();

// ...with:
var e = lastModule.get('$rootElement');
e.remove();

// ...the memory leak mostly goes away

Maybe @mgol knows more about it.

BTW, if you want to destroy an app, you better also destroy its root scope. It doesn't make any difference in your trivial example, but it a real application it will. I.e. in addition to removing $rootElement, also call $rootScope.$destroy().

Finally follow #8005 for an "official" way to destroy a bootstraped app.

Closing since every thing seems to work correctly (on the Angular side), but feel free to continue the discussion below.

@gkalpak gkalpak closed this as completed Oct 10, 2016
@djidel
Copy link
Author

djidel commented Oct 15, 2016

@gkalpak please check this: http://codepen.io/djidel/pen/vXjAqW?editors=0011
This basically bootstraps an app on a new DOM object and then destroy it many times. How many times and when to start creating them is controlled by the input text and the button respectively.
Please try to execute once and check the javascript memory usage then try 1000 times.
Does this count as memory leak?

@djidel
Copy link
Author

djidel commented Oct 15, 2016

@gkalpak also try to comment the 7th JS line (removing scope binding of the directive), this should lewer the memory use (on 1000 times).

@gkalpak
Copy link
Member

gkalpak commented Oct 15, 2016

I tried it and there is indeed a slight memory increase (~ 2.5MB in JS memory and another 2.5 in native memory for the first 1000 times). Removing the scope binding didn't make any difference for me.

TBH, I wasn't able to somehow relate this memory increase to Angular (but it may be due to my weak DevTools Timeline/Profile-foo). Both the JS Heap and Nodes seems to fall back to the initial sizes:

Before:
timeline1

After:
timeline2

Neither from the Heap Snapshot nor from the Allocation Timeline could I determine that the retained memory is due to Angular. According to my understand, most of the increase is caused by VM data related to deoptimization operations. (I could be wrong though.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants