'destroy' method is not cleaning up memory properly

[frozeman]

The slider works great in normal websites, but when you have a web app where the memory never gets refreshed because of page reloads and the slider is removed using the iosSlider('destroy') method, memory stacks up.

It seems that image load events and other events and items are not properly cleaned up in the destroy method, which renders this plugin unusable for modern web applications.

[marcwhitbread]

How many times are you destroying? Why are you destroying so many times? After destroying, what are you doing with the slider HTML? Reinitializing? I need more information about what you are trying to do.

The best the script can do is unbind the events it attaches, removing the CSS applied, and wiping out the jQuery data stored. All of this is being done on destroy.

I am always looking into performance improvements for the slider but there is no way to unload the images from memory without removing the image elements from the DOM altogether.

[frozeman]

I build a web app which uses your slider (we also bought a licencse) http://beta.tunedin.de Its a fat client side web app, means the page never reload, even when you're switching pages. Therefore every time you move to another page the template gets un-rendered, and i call the destroy method. When you switch pages many times, memory stacks up until 600mb (where around 100 is used by the web app itself). When i remove your slider the memory after garbage collection is around 100mb.

I also have some image load events going on in some part of my code. What i do is the following, to make sure the image is properly unloaded and cleared:

             var $img = $('<img src="'+ template.properties.src +'">');

            // fade in when loaded
            $img.on('load', function(e){
                var $img = $(this);
                // necessary to prevent memory leak
                $img.off('load');
               ....
            });

[frozeman]

I will also digg deeper to find the source of the leak.

[marcwhitbread]

Therefore every time you move to another page the template gets un-rendered, and i call the destroy method.

Meaning the elements are removed from the DOM?

When i remove your slider the memory after garbage collection is around 100mb.

Interesting. How are you calculating the amount of memory left over? What tools are you using?

What i do is the following, to make sure the image is properly unloaded and cleared:

Very interesting snippet of code. I will investigate this further.

I will also digg deeper to find the source of the leak

Great, let me know what you find. I know the memory problems are related to the images within the slider. But this has more to do with resizing the images already existing on the page based on slider size changes.

[frozeman]

Meaning the elements are removed from the DOM?

Yes. when templates get unrendern on page switches, the whole dom structure is removed by Meteor (most likely using jQuery.remove())

Interesting. How are you calculating the amount of memory left over? What tools are you using?

I use Chrome > Developer Tools -> timeline there you can see the memory stacking up, when you switch the page very often (you can also use a script for that, see below)

If you want to try it yourself with beta.tunedin.de run the following in the console

Meteor.setInterval(function () {
  Meteor.Router.to('/featured');

  setTimeout(function () {
    View.setViewLayer(false);
  }, 2500);
}, 4500);

I looked at your code and in the destroy methods, there are a few event which are attached to the window object which were not removed. I add the following to your destroy method. not sure if it cleans up properly as i'm not aware of you code in detail:

    var eventObject = $(window);    
if(isIe8 || isIe7) {
    var eventObject = $(document); 
}
    var orientationEvent = supportsOrientationChange ? 'orientationchange' : 'resize';
$(window).unbind(orientationEvent + '.iosSliderEvent-' + data.sliderNumber);
$(eventObject).unbind('mouseup.iosSliderEvent-' + data.sliderNumber);
$(window).unbind('scroll.iosSliderEvent');

I guess, like you say the main problem could be the images. I guess either attached events, or closures, where you use the image in an event function and i can never get cleaned up. You need then to null the variable.

I would love to see some improvements in this direction, otherwise i have to choose to build a more simple version myself, to prevent leaks.

[frozeman]

As reference you can use the following snippet to see how memory should behave on beta.tunedin.de:

 Meteor.setInterval(function () {
   Meteor.Router.to('/tvguide');

   setTimeout(function () {
     View.setViewLayer(false);
   }, 2500);
 }, 4500);

It will go up to ~200mb, but then it goes straight. When you would press the garbage collect button (the trash can upper left) you would be down to ~100mb

[marcwhitbread]

@frozeman thanks for the information. I will take a look at the tool and see if I can get to the bottom of the memory issues. Focussing on the initialization as the problem most likely resides there.

[marcwhitbread]

@frozeman have you taken a look at your heap snapshot after garbage collection? Looks like some of the memory problems are a result of detatched DOM tree nodes. These aren't being left behind in an iosslider only test. Have you taken a look at these orphaned nodes?

[frozeman]

Meteor's blaze, which is used for the template rendering takes care of the DOM nodes manipulation. Could be that it keeps them for a while.

Where do you noticed those and what kind of test setup you did create? Could you point me to the nodes, or the piece of code where you noticed that?

[marcwhitbread]

Where do you noticed those and what kind of test setup you did create? Could you point me to the nodes, or the piece of code where you noticed that?

I used your web page, let it refresh a few times using the script you provided, then forced garbage collection by clicking the trash can. Under the profile tab, take a heap snapshot. You will see all the detached nodes left behind.

[frozeman]

Thanks for this interesting lead. I will look into this.