davatron5000 / FitText.js

A jQuery plugin for inflating web type
http://fittextjs.com
6.75k stars 1.39k forks source link

Possible memory leak (jQuery event handlers cache) #117

Closed greenjoe closed 10 years ago

greenjoe commented 10 years ago

Hello! I'm using FitText in a single-page JavaScript application. I found out that my application has some memory leaks and I've been trying to track them down in Google Chrome developer tools.

Today I found out that large amount of objects in heap snapshots is located inside jQuery's cache objects. I paused the code execution in the debugger and I found a way to see what's inside the cache (I think that the jQuery's cache is not accessible from outside in any simpler way).

So the path I traversed is $ -> _removeData -> Closure -> data_priv -> cache -> 1 -> events And there I saw orientationchange: Array[6871] resize: Array[6871] (exact numbers depend on the number of .fitText() calls) All of objects in those arrays have a property namespace: "fittext" which led me to FitText.

The problem disappears after removing one line from the library code:

     $(window).on('resize.fittext orientationchange.fittext', resizer); 

It looks like it bounds a new event handler to window every time the method is executed, am I right?

A simple test case that causes the described behaviour is

        for(var i = 0; i<20000; i++){
            var text = $("<h1>").append("xyz");
            $(document.body).append(text);
            text.fitText();
            $(document.body).empty();
        }

I'm not an expert in JavaScript development and I'm not sure that it is a bug in FitText so I'm sorry if the issue is somehow invalid.

davatron5000 commented 10 years ago

I'll investigate this a bit further, but it seems the problem is that you've tried to apply FitText (which uses window.resize) to 6800+ elements. This is not recommended. window.resize is known to be a very expensive and memory intensive thing which is why we typically recommend it for headlines (see website fittextjs.com).

In your situation, with a single page app, you'll probably need to $(window).off('resize.fittext orientationchange.fittext') when you tear the view down and then reapply it when you build up. You'll have to do your own "garbage collection" if using this in a single page app.

greenjoe commented 10 years ago

Usually when the element is deleted using jQuery methods like $.empty() jQuery clears events cache and so on and there is no memory leak when you delete and re-create objects. I have seen that you recommend using FitText for headlines, but it isn't said anywhere that those headlines cannot be re-created periodically.

I think that it would be great if there were something like an unFitText() method that unbinds appropriate handler from window object so that you can safely delete objects that are "FitTexted". Otherwise I think that there's no way to delete an object that was once "FitTexted" without creating a memory leak (well you can of course execute $(window).off manually, but that requires depending on the library internals and not on its API).

From your answer I understand that this library is rather intended to be used in "permanent" headers so that of course makes this issue invalid.

Thank you for your answer and investigation.