19

I am building a single page webapp. This means over a period of time I get new DOM elements, remove unneeded ones. For example when I fetch a new form I just replace the contents of a specific div with that form HTML and also set up listeners unique to this form's elements. After some period I replace the contents of this form with a new instance of form (having different ID's).

I set up the event listeners again for this new form. Now the previous form is no longer part of the DOM so I the DOM elements should be automatically garbage collected. I am also expecting the listener functions pointing to the elements removed from the DOM to disappear.

However the following profile gathered from Chrome suggests that my listener count is increasing over time. Can you tell me why this is so? I tried clicking on the "Collect Garbage" button. But this is the profile I get. Is there something wrong with the way I am building my application? Is there a problem and if so how should I fix it?

Chrome snapshot of my webapp for a few minutes

In case it matters I am using JSP templating language with jquery, jquery-ui and some other plugins. This is how the dynamic fragments that I add/remove on my page look like.

<script>
  $(document).ready(function() {
    $("#unique_id").find(".myFormButton").button().click(
      function() {
        $.ajax({url: "myurl.html",
          success: function(response) {
                console.log(response);
          }
        });
    });
  });
</script>

<div id="unique_id">
    <form>
      <input name="myvar" />
      <button class="myFormButton">Submit</button>
    </form>
</div>

Update

If you want to have a look at the actual code here is the relevant portion. This link shows that when clear button is pressed the function clearFindForm is called which effectively refetches content (HTML fragment) using an ajax request and replaces the entire div in this jsp with the content fetched. The refetchContent function works as below: Here is the link to the code in case that helps in giving a better answer.

function refetchContent(url, replaceTarget) {
  $.ajax({
    url: url,
    data: {},
    type: "GET",
    success: function (response) {
       replaceTarget.replaceWith(response);
    },
    error:   function (response) {
       showErrorMessage("Something went wrong. Please try again.");
    }
  });
}
Michael Kohne
  • 11,888
  • 3
  • 47
  • 79
Rohit Banga
  • 18,458
  • 31
  • 113
  • 191
  • _Similar to .empty(), the .remove() method takes elements out of the DOM. Use .remove() when you want to remove the element itself, as well as everything inside it. In addition to the elements themselves, all bound events and jQuery data associated with the elements are removed._ – Ram Jan 20 '13 at 22:54
  • 1
    jQuery should automatically remove any bound handlers upon calling `.remove()`. Even if you use `.empty()`, jQuery will use `.remove()` internally. Not entirely sure how it goes for `.html()` – Fabrício Matté Jan 20 '13 at 22:55
  • I am using `.html(newContent)` and `.replaceWith()` for my webapp – Rohit Banga Jan 20 '13 at 22:56
  • 1
    Best memory solution, for your event problem, is to use event delegation instead of manually binding an event on individual elements. Yes it's slightly more work than individual event binding, but it guarantees only 1 handler versus guessing and hoping some functions are going to get GCed – adrian Jan 21 '13 at 02:09
  • but what is the root cause of this problem in my case? – Rohit Banga Jan 21 '13 at 02:27
  • There can be many causes of why your event count is going up. Not just one... The main one is probably events that never get undelegated. Look in you code closely where you define your events and then check that those events get removed. – Bruno Jan 24 '13 at 04:48

3 Answers3

5

While jQuery is very good at removing event listeners to DOM elements that are removed via it's methods (including .html() - just read the API: http://api.jquery.com/html/) - it won't remove event listeners to DOM elements that may still have a reference to them in a detached DOM tree.

For example, if you do something like this:

$.ajax({
    ....
})
    .done(function(response,status,jqXHR) {

        //create a detached DOM tree
        form = $(response)

        //add an event listener to the detached tree
        form.find('#someIDInTheResponse').on('submit',function() {

        });

        //add the form to the html
        $('#someID').html(form);
    });

//at some other point in the code
$('#someIDInTheResponse').remove();

Note that in the above example, despite the fact that you removed the element from the DOM, the listener will not be removed from memory. This is because the element still exists in memory in a detached DOM tree accessible via the global variable "form" (this is because I didn't create use "var" to create the initial detached DOM tree in the scope of the done function....there are some nuances and jQuery can't fix bad code, it can only do it's best.

2 other things:

Doing everything inside callbacks or event listeners (like do this on a button click) turns into real bad spaghetti code really fast and becomes unmanageable rather quickly. Try and separate application logic from UI interaction. For example, don't use callbacks to click events to perform a bunch of logic, use callbacks to click events to call functions that perform a bunch of logic.

Second, and somewhat less important, (I welcome feedback on this perspective via comments) I would deem 30MB of memory to be a fairly high baseline for a web app. I've got a pretty intensive google maps web app that hits 30MB after an hour or so of intensive use and you can really notice start to notice it's sluggishness when you do. Lord knows what it would act like if it ever hit 60MB. I'm thinking IE<9 would become virtually unusable at this point, although, like I said, I welcome other people's feedback on this idea.

Adam Jenkins
  • 51,445
  • 11
  • 72
  • 100
  • are you suggesting I should use `form.find('#someIDInTheResponse').on('submit',myFunction);` in place of `form.find('#someIDInTheResponse').on('submit',function() { });` – Rohit Banga Jan 21 '13 at 01:44
  • Added more information to the question so you can exactly see what's going on. Thank you. – Rohit Banga Jan 21 '13 at 01:57
  • @iamrohitbanga: No, he suggests to make the leaking global `form` variable local. – Bergi Jan 21 '13 at 01:59
  • I have used var form everywhere. There can be oversights but at most places I use var. – Rohit Banga Jan 21 '13 at 02:00
  • @iamrohitbanga This was just an example of how jQuery won't necessarily remove listeners when you remove DOM nodes. Ideally, you wouldn't attach the listener to the node (or even create the node) until it is part of the actual DOM, rather than create the node as a detached DOM tree and then append that detached tree to the DOM. Again, this is just one example that demonstrates how jQuery won't clean up everybody's messes. To see why your listeners keep going up, we'd need to see your app code. – Adam Jenkins Jan 21 '13 at 10:46
  • @Adam Thanks for responding. The app is open source just in case that helps you to answer the question better. Here is the source code https://github.com/C4G/V2V. The portions of your interest could be http://bit.ly/V0svZD and http://bit.ly/V0sNQ9. Does that help? – Rohit Banga Jan 21 '13 at 17:59
  • There are more details in the question above. I updated it. If you have any questions do let me know. Meanwhile I will learn how to use the chrome profiling better, may be that will help me debug it. – Rohit Banga Jan 21 '13 at 18:00
1

I wonder if you are simply not unbinding/removing the previously bound event listeners when you replace fragments?

I briefly looked at the specific sections of code you linked to in your updated question, but didn't see any event listener binding other than what you are doing in document ready, so I'm guessing you are doing some additional binding when you replace the document fragments. I'm not a jQuery expert, but in general binding or assigning additional event listeners does not replace previously bound/assigned event listeners automatically.

My point is that you should look to see if you are doing binding via "click()" (or via some other approach) to existing elements without unbinding the existing event listener first.

You might take a look at moff's answer to this question, which provides an example for click, specifically.

Community
  • 1
  • 1
ajh158
  • 1,477
  • 1
  • 13
  • 32
  • I query the server get DOM elements with unique IDs in the response. Now I register event handlers for these new DOM elements. When I query the server again I simply replace the old DOM elements with new DOM elements having different IDs. I do not unregister the old events. I believe that since these elements have been replaced from the DOM using jquery replaceWith() the event handlers must be automatically removed. I then register new event handlers in document.ready for these new DOM elements. – Rohit Banga Jan 30 '13 at 17:40
  • Thanks for looking at the code sample. But my assumption is that calling unbind should not be required as I am removing the element from the DOM using jquery. – Rohit Banga Jan 30 '13 at 17:54
1

I can't add a comment because of reputation but to respond to what Adam is saying...

To summarise the case Adam presents, it's potentially nothing to do with jQuery, the problem may be within normal Javascript. However you don't present enough code for anyone to really get to the bottom of the problem. Your usage of scoped encapsulation may be perfectly fine and the problem may be else where.

I would recommend that you search for tools for finding the cause of memory leaks (for example, visualising/traversing the entire object/scope/reference/function tree, etc).

One thing to watch out for with jQuery are plugins and global insertions into the DOM! I've seen many JS libs, not just jQuery plugins, fail to provide destroyers and cleanup methods. The worst offenders are often things with popups and popouts such as date pickers, dialogs, etc that have a nasty habit of appending layer divs and the like into body without removing them after.

Something to keep in mind if that a lot of people just get as far as to make things construct but don't handle destruct especially in JS because they expect still even in this day and age that you will be serving normal webpages. You should also check plugins for destroy methods because not all will hook onto a remove event. Events are also used in a messy fashion in jQuery by others so a rogue handler might be halting the execution of following cleanup events.

In summary, jQuery is a nice robust library but be warned, just because someone depends on it does not mean it inherits jQuery's quality.

Out of curiosity... have you checked the listeners on document.ready? Maybe you need to manually GC those.

jgmjgm
  • 4,240
  • 1
  • 25
  • 18