From 4c68d391dd1d218a69e0f211ffbb503f30d9eb62 Mon Sep 17 00:00:00 2001 From: Cristian Carlesso Date: Thu, 12 Apr 2012 20:31:13 +0200 Subject: [PATCH 1/4] Fix IE leaks caused by ._fireEvent circular reference changed empty, now it uses destroy to clean every children tree this change need a check on destroy, because childNodes returns textNodes on dispose i removed the _fireEvent reference breaking the circular reference and allowing the gc to clean up the memory (tested with sIEve-0.0.8) set('html',...) don't leak but set('text',...) does, so added a Element.Properties.text setter and done empty() before setting the text I setup a test page that creates 1000 divs, added these divs to the body and then use empty(), set('html', ...) and set('text', ...) to get rid of them, causing a 1000 element leak, with these changes I managed to drop the leaks from 1000 to 0. --- Source/Element/Element.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Source/Element/Element.js b/Source/Element/Element.js index e35f68b11..4d3008b09 100644 --- a/Source/Element/Element.js +++ b/Source/Element/Element.js @@ -800,6 +800,7 @@ var formProps = {input: 'checked', option: 'selected', textarea: 'value'}; Element.implement({ destroy: function(){ + if(! this.getElementsByTagName) return null; //textNode var children = clean(this).getElementsByTagName('*'); Array.each(children, clean); Element.dispose(this); @@ -807,11 +808,12 @@ Element.implement({ }, empty: function(){ - Array.from(this.childNodes).each(Element.dispose); + Array.from(this.childNodes).each(Element.destroy); return this; }, dispose: function(){ + this._fireEvent = null; return (this.parentNode) ? this.parentNode.removeChild(this) : this; }, @@ -949,6 +951,13 @@ Element.Properties.html = { }; +// fix for IE leak on Element.set('text','') +Element.Properties.text = { + set: function(text){ + Element.prototype.empty.call(this).setProperty('text',text); + } +} + var supportsHTML5Elements, supportsTableInnerHTML, supportsTRInnerHTML; /**/ From 14f5deb437a9db8e2e28e04b0d0eb8ce0451d617 Mon Sep 17 00:00:00 2001 From: Cristian Carlesso Date: Thu, 12 Apr 2012 20:37:05 +0200 Subject: [PATCH 2/4] Revert "Fix IE leaks caused by ._fireEvent circular reference" This reverts commit 4c68d391dd1d218a69e0f211ffbb503f30d9eb62. --- Source/Element/Element.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Source/Element/Element.js b/Source/Element/Element.js index 4d3008b09..e35f68b11 100644 --- a/Source/Element/Element.js +++ b/Source/Element/Element.js @@ -800,7 +800,6 @@ var formProps = {input: 'checked', option: 'selected', textarea: 'value'}; Element.implement({ destroy: function(){ - if(! this.getElementsByTagName) return null; //textNode var children = clean(this).getElementsByTagName('*'); Array.each(children, clean); Element.dispose(this); @@ -808,12 +807,11 @@ Element.implement({ }, empty: function(){ - Array.from(this.childNodes).each(Element.destroy); + Array.from(this.childNodes).each(Element.dispose); return this; }, dispose: function(){ - this._fireEvent = null; return (this.parentNode) ? this.parentNode.removeChild(this) : this; }, @@ -951,13 +949,6 @@ Element.Properties.html = { }; -// fix for IE leak on Element.set('text','') -Element.Properties.text = { - set: function(text){ - Element.prototype.empty.call(this).setProperty('text',text); - } -} - var supportsHTML5Elements, supportsTableInnerHTML, supportsTRInnerHTML; /**/ From 86611beefad82d50a9b2a9b19647d39ede914c1c Mon Sep 17 00:00:00 2001 From: Cristian Carlesso Date: Fri, 13 Apr 2012 07:42:23 +0200 Subject: [PATCH 3/4] Revert "Revert "Fix IE leaks caused by ._fireEvent circular reference"" This reverts commit 14f5deb437a9db8e2e28e04b0d0eb8ce0451d617. --- Source/Element/Element.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Source/Element/Element.js b/Source/Element/Element.js index e35f68b11..4d3008b09 100644 --- a/Source/Element/Element.js +++ b/Source/Element/Element.js @@ -800,6 +800,7 @@ var formProps = {input: 'checked', option: 'selected', textarea: 'value'}; Element.implement({ destroy: function(){ + if(! this.getElementsByTagName) return null; //textNode var children = clean(this).getElementsByTagName('*'); Array.each(children, clean); Element.dispose(this); @@ -807,11 +808,12 @@ Element.implement({ }, empty: function(){ - Array.from(this.childNodes).each(Element.dispose); + Array.from(this.childNodes).each(Element.destroy); return this; }, dispose: function(){ + this._fireEvent = null; return (this.parentNode) ? this.parentNode.removeChild(this) : this; }, @@ -949,6 +951,13 @@ Element.Properties.html = { }; +// fix for IE leak on Element.set('text','') +Element.Properties.text = { + set: function(text){ + Element.prototype.empty.call(this).setProperty('text',text); + } +} + var supportsHTML5Elements, supportsTableInnerHTML, supportsTRInnerHTML; /**/ From f59f069d20ea44b8496333f56eef8c012ae34a6c Mon Sep 17 00:00:00 2001 From: Cristian Carlesso Date: Fri, 13 Apr 2012 18:09:32 +0200 Subject: [PATCH 4/4] Applied the suggestions revert dispose, destroy now filter nodeType, so Element.properties.text can use empty without throwing errors, delete properties in clean. --- Source/Element/Element.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Source/Element/Element.js b/Source/Element/Element.js index 4d3008b09..20d9fbf4b 100644 --- a/Source/Element/Element.js +++ b/Source/Element/Element.js @@ -792,6 +792,11 @@ var clean = function(item){ delete collected[uid]; delete storage[uid]; } + var props = ['_fireEvent','$family','$constructor']; + props.each(function(property){ + if(property in item) delete item[property]; + }); + return item; }; @@ -800,10 +805,11 @@ var formProps = {input: 'checked', option: 'selected', textarea: 'value'}; Element.implement({ destroy: function(){ - if(! this.getElementsByTagName) return null; //textNode - var children = clean(this).getElementsByTagName('*'); - Array.each(children, clean); - Element.dispose(this); + if(this.nodeType == 1){ + var children = clean(this).getElementsByTagName('*'); + Array.each(children, clean); + } + Element.dispose(this); return null; }, @@ -813,7 +819,6 @@ Element.implement({ }, dispose: function(){ - this._fireEvent = null; return (this.parentNode) ? this.parentNode.removeChild(this) : this; }, @@ -954,7 +959,7 @@ Element.Properties.html = { // fix for IE leak on Element.set('text','') Element.Properties.text = { set: function(text){ - Element.prototype.empty.call(this).setProperty('text',text); + this.empty().setProperty('text',text); } }