Bug Tracker

Opened 12 years ago

Closed 12 years ago

#10682 closed bug (fixed)

Creating DOM elements with $(' ') leaks memory and skips the fragment cache

Reported by: niek.sanders@… Owned by: Rick Waldron
Priority: blocker Milestone: 1.7.1
Component: core Version: 1.7
Keywords: Cc:
Blocked by: Blocking:


Creating DOM elements with $() can cause memory usage to increase in a leak-like manner.

Full fiddles to reproduce this bug are given below. In brief... every time this routine is called, jQuery sucks up more memory:

  for ( var j = 0; j < 10; j++ ) {
    $('<li>' + j + '</li>').appendTo( fred$ );

I'm guessing the problem lies in the selector cache. If we run the exact same loop again, it's not finding the cached results. This code, in contrast, does not have memory-growth issues:

  for ( var j = 0; j < 10; j++ ) {
    $('<li></li>').appendTo( fred$ );

I've also found that using html() can work around the bug:

  for ( var j = 0; j < 10; j++ ) {
    $('<li></li>').html(j).appendTo( fred$ );

Here are two fiddles which illustrate the issue:

To reproduce, take heap snapshots in Chrome. Everytime the "Show Bug" button in clicked in the leaky version, the heap usage will increase by about 1.5 MB. The "good" version will have constant memory usage under the same test.

I'm using Linux and can reproduce the issue in both Chrome 15.0.874.106 and Firefox 7.0.1. For Chrome, I used the heap snapshots after each click. For Firefox, I used the system resource monitor to see the memory usage change.

Change History (11)

comment:1 Changed 12 years ago by Rick Waldron

Resolution: invalid
Status: newclosed

The memory consumption in this pattern will be the same with or without jQuery. Create a cached LI and clone it in each iteration. The second thing is to append new elements to a dummy fragment in the loop then move the to the real target after the loop ( this will reduce reflows)

comment:2 Changed 12 years ago by anonymous

So doing: 1) Create ten elements 2) Delete those ten elements 3) Go back to step 1

Resulting in constantly increasing memory usage is expected behavior? Yipes.

comment:3 Changed 12 years ago by dmethvin

Any sufficiently advanced cache algorithm is indistinguishable from a memory leak. :-) Processing HTML into DOM elements is expensive, which is why we cache HTML fragments and their corresponding DOM elements.

If you create ten *unique* html fragments smaller than 512 bytes then the fragment cache will have ten new entries. A line of code like $("<li />").attr("data-abc", n) creates only one entry in the cache even if executed repeatedly but $("<li data-abc='"+j+"' />") creates a cache entry for each unique value of j. Which is what you have verified.

So we have some options here. We can document the best practice of not creating lots of unique HTML strings, or we can have a way to shut off the fragment cache so your unique HTML strings won't be cached but your code will run slower. Given that you're generating a LOT of these, I suspect the page is already pretty large and slow. So it seems like documenting the behavior of the fragment cache and advising the style in your second example would be best. Do you agree?

As a hacky and unsupported workaround you could try setting jQuery.fragments = {} since that will empty the cache, but that might cause worse performance issues than the memory problems it solves.

comment:4 Changed 12 years ago by niek.sanders@…

Your caching explanation makes perfect sense.

There is just one small thing that troubles me.... if you look at the js Fiddle, there are only ten unique strings being fed to jQuery. It should be hitting the cache after each iteration of the loop yet it seems to be creating new cache entries.

comment:5 Changed 12 years ago by dmethvin

Component: unfiledcore
Milestone: None1.7.1
Priority: undecidedblocker
Resolution: invalid
Status: closedreopened
Summary: Creating DOM elements with $(' ') leaks memory.Creating DOM elements with $(' ') leaks memory and skips the fragment cache

Well just to be sure I put some debugging in the fiddle to dump the fragment cache ... and it's empty! So we have a different bug here, it looks like the condition to check for HTML5 validity in IE is borking things.


So we'll have to get that fixed before we can take it further. I'm reopening the ticket.

comment:6 Changed 12 years ago by anonymous

I ran your fiddle in both Chrome and Firefox on Linux. Empty fragment cache on both of those too.

"jQuery.fragments has 0 entries with 0 key bytes"

(both before and after the loop)

comment:7 Changed 12 years ago by Timmy Willison

Status: reopenedopen

comment:8 Changed 12 years ago by Rick Waldron

I'm willing to bet that the rnoshimcache condition can be removed now that we're fake cloning elements (in my pending patch)

comment:9 Changed 12 years ago by Rick Waldron

My previous suggestion is incorrect, looking for a different fix and writing expected jQuery.fragments tests.

comment:10 Changed 12 years ago by Rick Waldron

Owner: set to Rick Waldron
Status: openassigned

comment:11 Changed 12 years ago by Timmy Willison

Resolution: fixed
Status: assignedclosed

Remove test of the invalid object for IE9's sake; Rewrite the appropriate support test for html5 clone caching. Fixes #10682

Changeset: 41b31d7386cf00e714d703db773ffa73b6bd8b24

Note: See TracTickets for help on using tickets.