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

Memory leak in state change #1096

Closed
perrygovier opened this issue Apr 9, 2014 · 92 comments
Closed

Memory leak in state change #1096

perrygovier opened this issue Apr 9, 2014 · 92 comments
Assignees

Comments

@perrygovier
Copy link
Contributor

It appears that changing state increases memory usage with every transition. Here's a simple example. Open your browser's memory profiler and click "Get Clicky".

http://codepen.io/perrygovier/pen/FJgBa

Pages with more stuff on it tend to increase the amount of memory that gets eaten with each transition. Note the number of nodes and listeners never decreases.

screen shot 2014-04-09 at 5 31 45 pm

On my app that uses a few larger lists, the app can quickly get to 200mb after just a few mins of manual clicking back and forth. I have yet to crash the app due to memory pressure, but it seems more a matter of impatience than something that wouldn't happen with continued use.

I believe this is less an issue with Ionic and more to do with Angular's UI Router, but I thought I'd start here as it's where it popped up for me.

BountySource discussion that appears related

@adamdbradley
Copy link
Contributor

angular-ui/ui-router#545

@perrygovier
Copy link
Contributor Author

There's a parallel discussion that lead to this ticket being opened. It appears other users are seeing what I am in where larger apps escalate quicker.

http://forum.ionicframework.com/t/memory-leak/2823/4

More interesting, Aritzg created an example in pure Angular, and while memory spiked, it was quickly garbage collected. I'll try to whip up a demo this weekend to confirm. Perhaps there's something Ionic's doing to prevent or limit that GC event?

@perrygovier
Copy link
Contributor Author

I've done a bit more digging, but it's still a mystery what the root cause of the issue is. I ran some benchmarks against a plain UI Router example, matching the version Ionic is using. It does appear that the leak in ng-animate has been fixed.

This is code I used for these tests https://dl.dropboxusercontent.com/u/9006564/test.zip

First, without ng-animate
screen shot 2014-04-13 at 11 31 29 am

Then, with ng-animate
screen shot 2014-04-13 at 11 33 41 am

Note the immediate drop in memory after each page transition using ng-animate, that's to be expected. Both are reduced back to near their original size once the CG button is clicked at the end. The stepping in the first one was automatic GC before I was able to click the button.

It has been suggested on the UI Router github tickets, that the node and listener count perpetually increasing is actually a bug in web inspector, and can be ignored. Unfortunately, Safari and Firefox's inspector do not offer node or listener count to confirm this.

So UI Router may not be the sole cause of the leak. Taking a closer look at Ionic:

Here's a heap comparison (delta) going from the main page to another and back on the above UI Router demo, and the Ionic tabs demo.

Angular UI Router example
screen shot 2014-04-13 at 1 19 49 pm
Here's the Ionic tabs demo
screen shot 2014-04-13 at 1 21 43 pm

These lists can be pretty intimidating to sift through. Worth noting however, is that the node list has a zero delta in plain UI Router, and has grown by 2240 elements in Ionic. Now, after the first dashboard -> friends -> dashboard cycle, this could be just caching. However, this is after the third cycle, so I think the node delta should be 0.

More interesting are the elements in the Detached DOM Tree arrays. In it, I find markup from the previous pages. These detached DOM tree elements explain why having larger pages, large graphics, or video exacerbate the leak. I believe this is the primary culprit for the memory leaks that have gotten to be in the hundreds of MBs.

screen shot 2014-04-13 at 2 47 55 pm

Upon selecting any element in this list, I believe things that show up in yellow in the window below, indicate a reference keeping them from being released. Many of these reference createEventHandler() of the jQLight library. As plain Angular uses this too, I'll have to think about next steps in tracking this down. Maybe set up a log for that method, and then cross reference the elements with the list of elements not being released.

The search continues.

Hope this was helpful.

TLDR:
It looks like UI Router has fixed most of its problems, and the others are the inspector's fault. It appears that some event listeners aren't removed on state changes, keeping those old elements in memory even after they're removed from the DOM.

@adamdbradley
Copy link
Contributor

This is great, thanks for digging into this. So you said UI Router has fixed most of its problems, so do you mean its fixed its problems in version 0.2.7, or 0.2.10? Right now Ionic is using 0.2.7 because at the time 0.2.8 had bugs and we've been holding back and upgrading.

@aritzg
Copy link

aritzg commented Apr 30, 2014

Hi. I have just updated to 1.0 beta.2 and replaced ui-router 0.2.7 with 0.2.10 but still the memory leak remains.

@ajoslin
Copy link
Contributor

ajoslin commented Apr 30, 2014

It currently is not leaking for me in Chrome 34.0.18 OSX, given the example in the OP and using ui-router 0.2.10 with the nightly build.

Here's a codepen with 0.2.10: http://codepen.io/ionic/pen/fa145096b86fc28c81006da84028e28e/

@perrygovier
Copy link
Contributor Author

Still seeing a number of detached DOM trees, but only a single stray event listener, and the memory impact after initial caching and GC is negligible. Might be a good idea to test with more complicated templates, but it's looking good to me.

@ajoslin
Copy link
Contributor

ajoslin commented Apr 30, 2014

We should definitely get those stray event listeners then!

How can you tell with more clarity what is leaking? I don't really know how to use the memory profiler besides to look at the graph.

@aritzg
Copy link

aritzg commented Apr 30, 2014

I will try to reproduce my case in CodePen. Until now, the bigger the dom objects were (say images, item lists with images), the bigger the memory leak was.

@perrygovier
Copy link
Contributor Author

I don't understand it completely either, but in profiles, you can take multiple heap snapshots and then change from the summary view to the comparison view, seeing the changes in memory between two points. As you dig down, I'm not sure how to interpret everything, but I know detached DOM trees are DOM elements that are no longer in the DOM but are either waiting for GC or are being held on to by stray listeners or something similar.

After clicking through each page and populating the cache, it's my understanding there shouldn't be any additional detached DOM trees. Still, this is substantially less of a problem than it was.

@aritzg
Copy link

aritzg commented Apr 30, 2014

I have just forked @ajoslin 's pen

http://codepen.io/aritzg/pen/beaFy

As you will notice, I added a fourth tab called 'list' which shows a 6 item list. each item has a name, a description and an image.

I you try the "get clicky" button and start a record (In Chrome ) Developer tools -> Timeline --> Memory , you will see used memory will grow. From time to time, GC will do its job but memory baseline will keep growing.

Regards!

@FrancoAA
Copy link

FrancoAA commented May 1, 2014

Any update about this issue? Because I tried upgrading router-ui but the memory leaking persists

@ajoslin
Copy link
Contributor

ajoslin commented May 12, 2014

Working on it.

@ajoslin ajoslin reopened this May 19, 2014
@ajoslin
Copy link
Contributor

ajoslin commented May 19, 2014

I just fixed a memory leak: scrollView was adding listeners to the document and never removing them.

There might still be more leaks, though.

Could you guys investigate with the newest nightly (once it's up, about 30 minutes after this post)?

Thanks all.

@robdmoore
Copy link

Cool! @graemefoster and I suspected this today while profiling our app. Good to see a fix has been put in. I'll give it a whirl later in the week and see what impact it has.

@aritzg
Copy link

aritzg commented May 21, 2014

HI @ajoslin . First of all thanks for your efort.

I forked the codepen published before.

With 1.0.0 b3
http://codepen.io/aritzg/pen/oxdaf

With nightly
http://codepen.io/aritzg/pen/ohFyq

I benchmarked both but unfortunately IMHO, the memory leak remains.

Here the results.

benchmark

Blue marks refer to the moment when I click on "get licky".
Grean mark refer to the moment when I force GC manually

I am java/web developer and not very skilled with JS so, by now, I can not offer much more help than this.

Hope this helps.

Regards

@ajoslin
Copy link
Contributor

ajoslin commented May 22, 2014

Hi @aritzg,

Thanks for the benchmarks.

We'll continue to look at this.

@thechrisroberts
Copy link

I'm seeing a pretty nasty leak that I assume is related. I've got an app with a side menu and four tabs. First tab contains a Mapbox embed. Switch between tabs and memory grows and grows. This is particularly pronounced with the mapbox view.

I've thrown together a Codepen with a similar structure showing the effect. The mapbox view is pretty basic with no pins and the other tabs are minimal so the memory growth is not much (though any leak should be plugged) but in my actual app it is quite serious, leading the app to crash after a short period of usage, rendering it essentially unusable.

http://codepen.io/thechrisroberts/pen/IowtF

The image below shows snapshots after the initial load, then switching to tab 2 and tab 3, then back to tab 1. Memory use inches up every step.

screen shot 2014-06-10 at 9 33 06 am

The next image shows the app I'm working on: initial load, switch to a couple of tabs, and switch back to the tab with the map.

screen shot 2014-06-10 at 9 35 09 am

@ajoslin
Copy link
Contributor

ajoslin commented Jun 10, 2014

Hi @thechrisroberts,

Thanks for the information. Does the mapbox have a 'destroy' or 'cleanup' method you can use?

For example:

var map = /* ... */
$scope.$on('$destroy', function() {
  map.destroy();
});

@thechrisroberts
Copy link

It does, just updated the pen to use it. I experimented with that in my app and it didn't seem to make a difference but it looks like it does help in the pen. I'll look at it a bit more in my app.

@thechrisroberts
Copy link

Just did a little more experimenting with my app. In addition to Mapbox I'm using markerclusterer. Destroying the mapbox did not destroy the markers; I had to remove those before removing the map. Saves a nice chunk of memory, but I still have memory growing when I switch between my other tabs which use fairly straightforward lists: image, text, that's it.

@aritzg
Copy link

aritzg commented Jun 12, 2014

Hi @ajoslin

I benchmarked again Ionic 1.0 beta.3 VS Nightly (20140612)

image

As shown in the image above there is a great improvement regarding memory leak.

Now, after the "get Clicky" process,once I force the GC, the memory usage falls.

But after repeating the process several times, the memory baseline keeps increasing 4 to 5 MB in each iteration, wich is way better than it was in previous versions.

I think it is still not resolved completely but it is almost there.

Congratulations!

@bappy004
Copy link

I still see around 1.1MB memory leak when using the tabs app and a map inside one of the tabs. Using 1.0.0 b7. Is there any update on this issue?
Thanks.

@sdhull
Copy link

sdhull commented Jan 14, 2016

FWIW, I was unable to repro in just Chrome and I don't really know how to profile in iOS. I tried the Profiler tool but it measures at the Objective-C level, so not sure what I'm even looking at. Poking at my app produced a few object leaks but they measured like 50-150 Bytes. Total "Persistent Bytes" seemed to stay in control ranging between 30-55mb.

@koolpitt
Copy link

koolpitt commented Feb 4, 2016

Hello,

Please any solution to this issue?

@surfjedi
Copy link

I have a solution, ditch ionic! Use something like https://onsen.io/ I have used and it super reliable and fast!

@plnkr2015
Copy link

Ionic team, please help. My app is clocking 150mb++ in android. App has tabs in it. Please help.

@christoph-thommen
Copy link

I think my ticket might be related to this one...
#5743

Same issue here... for long running apps, this behaviour is really a pain :-(

@mattgrothmove
Copy link

For those still experiencing issues it may be worth trying the following, it helped us get our app back into a usable state after transitioning between views back and forth as many of you indicate is your test scenario.

The following timeline is from transitioning 5 times back and forth, waiting 5 seconds, and repeating 5 times. You can see there is almost no memory freed up - which indicates something is being left in memory between the views.
broken

We started noticing that after going from a list of items, into the item detail view and back out again - over and over, eventually the browser would simply stop responding. The memory usage would climb each successive action, at a steady rate. We investigated a number of possible options (which you should do as well as your results may vary here...check the usual suspects - rogue listeners, use one-time binding when possible. If you are using an extension like Batarang or similar to debug angular in anyway, disable them! They will continually monitor watchers in your app and actually cause fake memory leaks (while active) by holding the watchers in memory and block them from being garbage collected. This will present itself by your watchers count going up and up and up each time you navigate to/from a page and give you a false positive on your leak. So make sure you disable all extensions if possible.

What solution ended up working for us, was to manually clean up directive scopes by watching the elements $destroy event, and manually calling the $destroy event on the child scope. This incredibly simple fix (I am calling it a fix, but its more of a band-aid solution) made our application timeline look a lot more like we expected. The following is how we implemented this. (in both the parent, and child directive)

element.on('$destroy', function () {
     // remove reference to this scope.
     scope.$destroy();
});

Below is the timeline after adding the code snippet to manually $destroy the scope.
better

So while this did seem to resolve the issue for us - I still suspect there is something else going on here, and while I will continue to investigate, the solution we came up with worked in a pinch for our timeline. Your mileage may vary. Hope it helps.

@jgw96 jgw96 added the bug label Mar 29, 2016
@sdhull
Copy link

sdhull commented Mar 29, 2016

@mattgrothmove very interesting indeed. So you added this to custom directives? I think https://github.com/vitalets/checklist-model is the only custom directive my app is using...

@Iodine-
Copy link

Iodine- commented Mar 30, 2016

@sdhull in my case we are not actually using ionic at all - I do not think this issue is related to Ionic directly but and underlying condition in another component. Which is why I thought to mention what we did.

Ours in an angular application that has several directives which we use ui router to manage states, which sounds similar to what you have in your ionic app. The fix for us was applied in the link method of the directives - but I am sure it could be adapted to be applied directly to a controller with a similar effect. I would be happy to take a look for you, if your project is something you are free to share with me, if not - I suggest you look in to your scopes and ensure they are being destroyed as expected. Would also be worth looking into you use of watchers, it really depends on how your app is implemented!

I know it can be a bit of a crap shoot just trying different things - but honestly, its the only way to really narrow down the root of the issue. Let me know if there is anything else I can do.

@sdhull
Copy link

sdhull commented Mar 30, 2016

@Iodine- ahhh I see. I guess I could probably do

    $scope.on("$destroy", function(){
      $scope.$destroy();
    });

In the controller... maybe even use $scope.$$childHead and then $$nextSibling to loop through all child scopes and destroy them too. I'll mess with this stuff a bit tomorrow.

Thanks for the help! Hopefully this (or similar) will work for us, though currently the memory issue hasn't (as far as I know) caused any problems for customers...

@Iodine-
Copy link

Iodine- commented Mar 30, 2016

@sdhull I am not positive the code you suggested is going to do much.

I would first start with the following to check if your controller scopes are indeed being destroyed or not. In my case, if I did not manually trigger the scope.destroy on the element.destroy event, then it would not trigger. See if you have a similar issue first.

$scope.$on('$destroy', function() {
     console.log('scope is being destroyed');
});

Second, I would suggest you take a look at your bindings and make sure you've removed any listeners you have on those controllers, especially those watching on $rootScope, which can hold child scopes open. Then go through and use one-time data binding on data that will not likely change in short timeframes (titles, names, phone numbers...etc) by swapping out {{customer.name.given}} with {{::customer.name.given}} to cut down on the watchers there too where possible.

I suggest you change 1 thing at a time, and run a benchmark before you start and after each change you make to check its affect (if any). My test was to navigate to a page and back 5 times, then wait 5 seconds (for timeouts and GC) and repeat that 4 times. Check your timeline and pay attention to your heap size and nodes.

Let me know if there is anything else I can do.

@cenobyte321
Copy link

I've been struggling for some time now with this nasty issue. My app has a view with a PDF, rendered with PDF.js and a couple of videos, rendered with the video tag. In iOS the memory is leaking a lot when going back and forth from this view. After doing this a couple of times the app crashes. I haven't been able to pinpoint exactly where the leak is. If anyone is kind enough to lead me into the right direction for solving this, I would greatly appreciate it.

Here's the project with only the problematic view: https://github.com/cenobyte321/ionic-pdf-video-memory

And a screenshot showing the leak in iOS moments before crashing:

screen shot 2016-04-05 at 3 26 35 pm

@jgw96 jgw96 reopened this Apr 5, 2016
@jgw96 jgw96 closed this as completed Apr 5, 2016
@Iodine-
Copy link

Iodine- commented Apr 5, 2016

@cenobyte321 I believe i've tracked down your leak to somewhere in the PDF module you are using. When I benchmark the app without the PDF module the garbage collector can cleanup almost everything. With the module included it doesn't even get 40%. Notice how at the end of the second test the memory usage appears to return to where it was on the first transition, that's what you want to see.

With PDF Module
screen shot 2016-04-05 at 3 24 59 pm

Without PDF Module
screen shot 2016-04-05 at 3 23 43 pm

Hope that helps.

@cenobyte321
Copy link

@Iodine- Thank you for your reply! Deleting the PDF module did reduce the memory leak, but I believe there's something else going on. By using the script I posted in the repository's readme file the memory leaked to a lesser degree, but after about 400 iterations of the script the app crashed. I know that might be an exaggeration but I could see the app still consuming memory and not releasing it, as shown in the XCode Debugger. The code I used is in the branch 'test-1-no-pdf': https://github.com/cenobyte321/ionic-pdf-video-memory/tree/test-1-no-pdf

Here's the iOS memory graph before the crash:
screen shot 2016-04-05 at 7 33 21 pm

EDIT: Just as an additional sanity check I removed the $timeout calls in the view's controller and manually tested the app, it still doesn't free up memory:

screen shot 2016-04-05 at 8 13 56 pm

@jrianto
Copy link

jrianto commented Apr 13, 2016

My application is a simple pages with LOTS of videos on it. Navigating back and forth crashes the app eventually, before it crashes the app, it will stop loading the videos first.

Can you check on my code and let me know how to destroy everything that is not in the current page from memory? I am new to Ionic, and not sure how to destroy/clean everything from the memory except the current page that is being viewed.

Please check my source at http://flei.ca/FLEI33.zip

If you load it on Google Dev Tools with ionic serve, then navigate back and forth, you will see the video will stop loading eventually. On an actual iPhone after installing the IPA to it, the apps crashes after a while and closes out.

Thank you very much for your help!

@ghost
Copy link

ghost commented Apr 25, 2016

@jrianto I had this issue on my Ionic app that uses embedded video. It seemed to be fixed by adding video.src ="" to the bottom of my code used when you change page/video

`.controller('ProductCtrl', function (Products, $rootScope, $scope, $stateParams, $state, $timeout) {
$scope.product = Products[$stateParams.productId];

var video = document.getElementById("myVideo");

// Stop video playing when we go to another page
$rootScope.$on('$stateChangeStart', function () {
stopVideo();
});

// Go to list of other videos when individual HTML5 video has finished playing
angular.element(video).bind('ended', function () {
$state.go('app.products');
});

function stopVideo() {
video.pause();
video.currentTime = 0;
video.src ="";
}
})`

@tslater
Copy link

tslater commented May 26, 2016

@jgw96, is there a fix for this, or should we reference another issue, or are you thinking these are all different problems rather than an ionic bug?

@jgw96
Copy link
Contributor

jgw96 commented May 26, 2016

Hello @tslater ! Thanks for the questions. So, I have done alot of testing on the issues discussed here and in every case it seemed to be related to memory leaks in external libraries/external code. I could not find a good case of a memory leak that seemed to be coming from ionic. Now, this is not me saying that Ionic is written perfectly memory safe, im not sure there is a piece of software out there that is perfectly memory safe, but it does not seem that Ionic is causing the above mentioned major memory leaks or even minor ones. Also, just to be thorough i have tested on older/low-end devices (Nexus 7 2013 and Iphone 5), mid range devices(Moto X 2014) and newer, very performant devices (Nexus 6). Hope this answers your question ! Thansk for using Ionic!

@sdhull
Copy link

sdhull commented May 26, 2016

@jgw96 wow that's very interesting. Did you see my comment from January? Have you tried out the repo I linked to there? Very vanilla implementation and easily reproducible memory problems. Right?

@kby799
Copy link

kby799 commented Aug 4, 2016

I have this issue for awhile now, but I thought it was the way I coded so I tried to change my data structure. Still the problem persists, but I notice the problem comes only when I try to go from one view to the next. Also how inconsistent it is, sometimes it is a short climb and some other time it is a big jump that it freezes up the app.

P.S I use tabs template to create this app, and without any manipulation from me. It also shows some kind of memory issue as well, happens between stages.

screen shot 2016-08-04 at 15 57 03

@vahidvdn
Copy link

Is there any fix? I have still the same problem.

@jrianto
Copy link

jrianto commented Feb 12, 2017

@vahidvdn on my case, it is a coding issue as I am very new to ionic. I had to unload the audio/video from memory before loading a new one. So every time my app loads a new video/audio, I am unloading the previously loaded one by doing:
videoElement.src =""; // empty source

That did the trick for my app and it never crashed again. So it's not an ionic issue, it's just me not knowing how to code it properly. Hope that helps.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 4, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 4, 2018
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