Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #4465 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Incredible memory leak in IE

Reported by: jaen@laborint.com Assigned to: sam
Priority: normal Milestone:
Component: Prototype Version:
Severity: critical Keywords:
Cc:

Description

The SVN version of Prototype (1.5.0_pre1) seems to leak every element passed to $, ie. practically everything.

Verified using Drip-0.3 from http://www.outofhanwell.com/ieleak/

Test script:

<html>
<head>
<title>Leak tester</title>
<script src="prototype.js" language="javascript"></script>
<script language="javascript">
function load() {
    var i;
    var fragment = document.createDocumentFragment();
    for(i = 0; i < 500; i++) {
        var el = document.createElement("span");
        el.appendChild(document.createTextNode(i.toString()+ " "));
        fragment.appendChild($(el));
    }
    document.body.appendChild(fragment);
}
</script>
</head>
<body onload="load()">
</body>
</html>

General memory usage increases roughly 60 times (!) with Prototype enabled, for 25M of memory Prototype can create 500 elements, while normal DOM methods yield 30000 elements. According to Drip, every element passed to $/Element.extend is leaked.

Reason: Element.extend creates a function closure that contains a reference to the element itself and stores it in the element's expando properties, creating a circular reference.

Culprit: (from dom.js)

element[property] = value.bind(null, element);

See http://msdn.microsoft.com/library/default.asp?url=/library/en-us/ietechcol/dnwebgen/ie_leak_patterns.asp for general advice, the method used by Prototype to extend elements is practically the first leak example there.

Fix: Remove Element.extend? Besides leaking it uses too much memory in any case due to the expando properties.

Change History

03/28/06 17:59:03 changed by jaen@laborint.com

Adding expando properties to every touched DOM node is a very bad idea in my humble opinion, but if that's absolutely required, there might be a way to fix it. Instead of creating closures, create a custom wrapper that's only created once:

function createWrapper(method) {
  // Does not account for extra args
  return function() {
    Element.Methods[method].call(null, this);
  }
}

wrappers = {'update': createWrapper('update'), ...};

function extend(element) {
  element['update'] = wrappers['update'];
  ...
}

03/28/06 19:09:52 changed by sam

Another idea is to implement a manual garbage collector for IE.

For each DOM element passed to Element.extend, add this element to a global object. Then at some interval (say every 60 seconds), iterate over all the object's properties (i.e., all the elements that have been extended) and check to see whether or not they're still in the DOM. If not, null all the added properties and remove the element's reference from the global object. We'd also do this on window.onunload.

Thoughts?

03/28/06 19:43:29 changed by Martin Bialasinski

IMHO, the first suggestion sounds more staight forward, simpler and less expensive. During the garbage collection, the browser is unresponsive. And IE is not the fastest in querying the DOM. Therefore, my vote is on no.1.

03/29/06 00:00:32 changed by sam

  • status changed from new to closed.
  • resolution set to fixed.

(In [4094]) prototype: Cache methods added to DOM elements with Element.extend to prevent memory leaks in IE. Closes #4465.

03/29/06 00:02:20 changed by sam

Thanks for the suggestion -- it was indeed a much simpler solution.

08/02/06 13:16:59 changed by anonymous

gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda gongda