#10682 closed bug (fixed)

Opened November 05, 2011 09:17AM UTC

Closed November 08, 2011 05:08AM UTC

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

Reported by:	niek.sanders@gmail.com	Owned by:	rwaldron
Priority:	blocker	Milestone:	1.7.1
Component:	core	Version:	1.7
Keywords:		Cc:
Blocked by:		Blocking:

Description

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:

  fred$.empty();
  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:

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

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

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

Here are two fiddles which illustrate the issue:

http://jsfiddle.net/MQZtY/6/ (leaky version)
http://jsfiddle.net/MQZtY/5/ (good version)

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.

Attachments (0)

Change History (11)

Changed November 05, 2011 04:59PM UTC by rwaldron comment:1

resolution:	→ invalid
status:	new → closed

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)

Changed November 05, 2011 05:10PM UTC by anonymous comment:2

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.

Changed November 05, 2011 07:09PM UTC by dmethvin comment:3

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.

Changed November 05, 2011 07:24PM UTC by niek.sanders@gmail.com comment:4

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.

Changed November 05, 2011 08:02PM UTC by dmethvin comment:5

component:	unfiled → core
milestone:	None → 1.7.1
priority:	undecided → blocker
resolution:	invalid
status:	closed → reopened
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.

http://jsfiddle.net/dmethvin/MQZtY/7/

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

Changed November 05, 2011 08:14PM UTC by anonymous comment:6

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)

Changed November 05, 2011 09:19PM UTC by timmywil comment:7

status:	reopened → open

Changed November 05, 2011 09:49PM UTC by rwaldron comment:8

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

Changed November 06, 2011 12:07AM UTC by rwaldron comment:9

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

Changed November 06, 2011 12:07AM UTC by rwaldron comment:10

owner:	→ rwaldron
status:	open → assigned

Changed November 08, 2011 05:08AM UTC by timmywil comment:11

resolution:	→ fixed
status:	assigned → closed

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

Changeset: 41b31d7386cf00e714d703db773ffa73b6bd8b24

Side navigation