[Prototype-core] Re: Memory Leak in IE
On Thursday 01 October 2009 21:56:30 Mike Rumble wrote: You could also encapsulate this in a function wrapping Element#remove, which IMHO is something Prototype should do out of the box. Quite disagree: * If I remove an element and add it elsewhere, I don't expect its events to have been de-registered. The code that moves the element shouldn't have to be aware of the (possibly unrelated) code that added the event listeners in order to ask it to add them again. * Removing from the document is not the same as allowing to be GC'd * Some elements may never be added to the document. Eg, an XML document which you download, manipulate then build some HTML representation of. Perhaps you want to monitor for mutations and keep the HTML in sync? [1] Jim [1] Not actually possible in IE or Chrome/Safari but would be nice if it were. In Chrome DOM mutation events only fire if the element is in the document: http://jimhigson.blogspot.com/2009/09/chrome-and-dom-mutation-events.html -- Jim my wiki ajaxification thing: http://wikizzle.org my blog: http://jimhigson.blogspot.com/ --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak in IE
Exact, an element removed may be add it elsewhere and my purpose is out It remains that the proposal of Mike element.descendants (). invoke ( 'stopObserving'); element.stopObserving (); But I find it rather heavy for the developer who must insert code to delete each element and can also cause performance's problems. Could not we consider making public the method _destroyCache and add it an optional parameter ancestor? Expl : function destroyCache(ancestor) { ancestor = $(ancestor); for (var i = 0, length = CACHE.length; i length; i++) { if (!ancestor) || (CACHE[i].descendantOf // Possible on Window / Document Elements (CACHE[i].descendantOf(ancestor) || CACHE[i].match(ancestor))) { Event.stopObserving(CACHE[i]); CACHE[i] = undefined; } } CACHE = CACHE.reject(Object.isUndefined); } This method could be integrated also at the AjaxUpdater before replacing the content ? Franck, On 2 oct, 10:06, Jim Higson j...@wikizzle.org wrote: On Thursday 01 October 2009 21:56:30 Mike Rumble wrote: You could also encapsulate this in a function wrapping Element#remove, which IMHO is something Prototype should do out of the box. Quite disagree: * If I remove an element and add it elsewhere, I don't expect its events to have been de-registered. The code that moves the element shouldn't have to be aware of the (possibly unrelated) code that added the event listeners in order to ask it to add them again. * Removing from the document is not the same as allowing to be GC'd * Some elements may never be added to the document. Eg, an XML document which you download, manipulate then build some HTML representation of. Perhaps you want to monitor for mutations and keep the HTML in sync? [1] Jim [1] Not actually possible in IE or Chrome/Safari but would be nice if it were. In Chrome DOM mutation events only fire if the element is in the document:http://jimhigson.blogspot.com/2009/09/chrome-and-dom-mutation-events -- Jim my wiki ajaxification thing:http://wikizzle.org my blog:http://jimhigson.blogspot.com/ --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak in IE
Ok, good points that I hadn't considered. However, I would think that many developers will just use Event#remove without considering the need to remove the event listeners, which could lead to memory leaks. Maybe an Element#destroy method could fill this gap - remove event listeners, remove element from the DOM and then trash it - a destructive method for when the developer says OK, I'm done with this element... On Oct 2, 9:06 am, Jim Higson j...@wikizzle.org wrote: On Thursday 01 October 2009 21:56:30 Mike Rumble wrote: You could also encapsulate this in a function wrapping Element#remove, which IMHO is something Prototype should do out of the box. Quite disagree: * If I remove an element and add it elsewhere, I don't expect its events to have been de-registered. The code that moves the element shouldn't have to be aware of the (possibly unrelated) code that added the event listeners in order to ask it to add them again. * Removing from the document is not the same as allowing to be GC'd * Some elements may never be added to the document. Eg, an XML document which you download, manipulate then build some HTML representation of. Perhaps you want to monitor for mutations and keep the HTML in sync? [1] Jim [1] Not actually possible in IE or Chrome/Safari but would be nice if it were. In Chrome DOM mutation events only fire if the element is in the document:http://jimhigson.blogspot.com/2009/09/chrome-and-dom-mutation-events -- Jim my wiki ajaxification thing:http://wikizzle.org my blog:http://jimhigson.blogspot.com/ --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak in IE
Element#destroy would definitely be useful. +1 I really think this is a better idea then making a public interface to reach destroyCache since it resolves into one function call and the learning curve (making difference between remove and destroy) is smaller since you don't have to teach developer the whole EventCache concept and why they should call the related function. Simon 2009/10/2 Mike Rumble mike.rum...@gmail.com Ok, good points that I hadn't considered. However, I would think that many developers will just use Event#remove without considering the need to remove the event listeners, which could lead to memory leaks. Maybe an Element#destroy method could fill this gap - remove event listeners, remove element from the DOM and then trash it - a destructive method for when the developer says OK, I'm done with this element... On Oct 2, 9:06 am, Jim Higson j...@wikizzle.org wrote: On Thursday 01 October 2009 21:56:30 Mike Rumble wrote: You could also encapsulate this in a function wrapping Element#remove, which IMHO is something Prototype should do out of the box. Quite disagree: * If I remove an element and add it elsewhere, I don't expect its events to have been de-registered. The code that moves the element shouldn't have to be aware of the (possibly unrelated) code that added the event listeners in order to ask it to add them again. * Removing from the document is not the same as allowing to be GC'd * Some elements may never be added to the document. Eg, an XML document which you download, manipulate then build some HTML representation of. Perhaps you want to monitor for mutations and keep the HTML in sync? [1] Jim [1] Not actually possible in IE or Chrome/Safari but would be nice if it were. In Chrome DOM mutation events only fire if the element is in the document: http://jimhigson.blogspot.com/2009/09/chrome-and-dom-mutation-events -- Jim my wiki ajaxification thing:http://wikizzle.org my blog:http://jimhigson.blogspot.com/ --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak in IE
Rather than hacking core consider doing something like... element.descendants().invoke('stopObserving'); element.stopObserving(); ..before removing your element from the DOM. You could also encapsulate this in a function wrapping Element#remove, which IMHO is something Prototype should do out of the box. For the future this kind of request is probably best posted in the general group. Mike. On Sep 30, 2:26 pm, kef f.watt...@gmail.com wrote: Hello, I am currently working on an AJAX application (prototype 1.6.1). As usual, it runs very well on Chrome and Firefox but we encounter difficulties, particularly with Internet Explorer 6. After much research, it appears that the problem is related to the memory leak. In fact, i reload only some parts of page. (I keep always the header and footer) so a large part of the memory used is never released. I found a solution partially responsive to my needs but it involves changing the core prototype. Prior to Ajax call, I clean all events on elements will be replaced (like _destroyCache during the unload of window). There may be a cleaner solution but I have not found. My changes here: remove: function (element) ( / / Begin Fix if (element = $ (element)) element.stopObserving (false, true); / / End Fix element.parentNode.removeChild (element); return element; ) stopObserving function (element, eventName, handler) ( element = $ (element); / / Begin Fix if (handler === true) ( for (var i = 0, length = CACHE.length; i length; i + +) if (CACHE [i]. descendantOf CACHE [i]. DescendantOf (element)) ( Event.stopObserving (CACHE [i], eventName); CACHE [i] = undefined; ) CACHE = CACHE.reject (Object.isUndefined); ) / / End Fix ) In view of the evolution of AJAX and implementation of Internet Explorer browsers, I think it may be advisable to incorporate a equivalent system in core of prototype. Regards, Franck --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak in IE
We've talked about doing this before, but there are a couple problems. First, we can't do what you're suggesting for Element#remove — the function returns the removed element, which may then be appended somewhere else in the document. If the element is going to be discarded, then one ought to remove its listeners; but if it will be reused, the user would expect those listeners to remain. I'm not sure how to get around this. We have also talked about removing event listeners on an Element#update call, but are hesitant to do so automatically for performance reasons. But we may yet implement this. Thanks! Andrew On Sep 30, 2009, at 6:26 AM, kef wrote: Hello, I am currently working on an AJAX application (prototype 1.6.1). As usual, it runs very well on Chrome and Firefox but we encounter difficulties, particularly with Internet Explorer 6. After much research, it appears that the problem is related to the memory leak. In fact, i reload only some parts of page. (I keep always the header and footer) so a large part of the memory used is never released. I found a solution partially responsive to my needs but it involves changing the core prototype. Prior to Ajax call, I clean all events on elements will be replaced (like _destroyCache during the unload of window). There may be a cleaner solution but I have not found. My changes here: remove: function (element) ( / / Begin Fix if (element = $ (element)) element.stopObserving (false, true); / / End Fix element.parentNode.removeChild (element); return element; ) stopObserving function (element, eventName, handler) ( element = $ (element); / / Begin Fix if (handler === true) ( for (var i = 0, length = CACHE.length; i length; i + +) if (CACHE [i]. descendantOf CACHE [i]. DescendantOf (element)) ( Event.stopObserving (CACHE [i], eventName); CACHE [i] = undefined; ) CACHE = CACHE.reject (Object.isUndefined); ) / / End Fix ) In view of the evolution of AJAX and implementation of Internet Explorer browsers, I think it may be advisable to incorporate a equivalent system in core of prototype. Regards, Franck --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Done, its here - http://prototype.lighthouseapp.com/projects/8886-prototype/tickets/425 Cheers, Phuah Yee Keat --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Hi, I think I need to clear some things up. After more testing, I have realized that there are two (quite) separate issues here: 1. purging of observers during removeChild/replaceChild/Element.update, preventing memory leaks within a single page 2. purging of all observers, _including those that are not in the DOM_, during page unload, preventing memory leaks across pages My temporary fix to my project, as well as the purgeObservers() method that have been suggested, attempts to fix issue #1. I see the fix to issue #1 as a feature request since there's no such feature before this. Now for issue #2, I see it as a bug in prototype-1.6, the reason being, that using my test program attached, you can observe the memory leak in IE (simply using the task manager will do) with prototype-1.6.0.3.js, but not with prototype-1.5.1.2.js. The change that created this memory leak is the removal of the stopObserving calls during page unload. I suggest putting the stopObserving calls back in, as it is obvious that simply setting cache[][]=null, is not a replacement for stopObserving(), it just works for preventing memory leaks on elements that are attached to the DOM during page unload. Simply uncommenting the document.appendChild(span) line in my test program will show that both prototype-1.5 and prototype-1.6 will not leak memory in IE. Cheers, Phuah Yee Keat --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~--- Title: Testing prototype observe
[Prototype-core] Re: Memory Leak on IE
Hi, Attached is the proposed patch against prototype-1.6.0.3. Cheers, Phuah Yee Keat --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~--- prototype-memory-leak.patch Description: Binary data
[Prototype-core] Re: Memory Leak on IE
On Oct 31, 5:05 am, Yee Keat Phuah [EMAIL PROTECTED] wrote: Hi, Attached is the proposed patch against prototype-1.6.0.3. Cheers, Phuah Yee Keat prototype-memory-leak.patch 1KViewDownload Could you create a ticket for this [1] and attach both files to it? It would be easier for us to keep track of it. Thanks. [1] http://prototype.lighthouseapp.com/projects/ -- kangax --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Hi, FWIW, I don't think this is a bug in Prototype. Prototype keeps track of the event handlers it hooks up so it can unhook them on page unload to work around memory leaks in IE. (This also lets it provide the new stopObserving functionality in release 1.6 where you don't have to specify the handler to unhook things.) This means that it has to use a small bit of memory to track the handlers. That's not a *leak*, I wouldn't say, but a *use* of memory. If you remove an element from the DOM (directly or by removing its ancestor), I think it's incumbent on your application logic to remove any handlers you've registered for it as well. Your function that does this with a select(*) is obviously one way to do that, but ideally there'd be something more finely-grained that app logic could handle. Personally, I wouldn't want to see the select(*) thing added to Prototype (for instance, in the Element.update method), because I don't want that overhead every time I remove an element if I know I don't have any handlers registered on it. Someday we may get access to the DOM event DOMNodeRemovedFromDocument[1]. Now that would be nice, because it would give us notification of an element being removed and let us release any memory we have associated with it. (I'm not sure I'd want Prototype to do it for me even then, but I'll make that call when/if we get the event.) [1] http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/ If it's very complicated in your app to know which elements within the element being replaced are being watched, you might look at event delegation instead, which involves fewer handlers placed at a higher (e.g., container) level. FWIW, -- T.J. Crowder tj / crowder software / com On Oct 30, 7:57 am, Yee Keat Phuah [EMAIL PROTECTED] wrote: Hi, Was trying to cut the memory leak reported with this tool -http://home.wanadoo.nl/jsrosman/. Found out that there are some memory leaks in prototype's Event.observe, only when the element that is observed is not within the document DOM, this might happen when 1. I have not attached the element into the DOM, or that 2. I have removeChild or replaceChild the element that was observed on. (this is what I encountered in my project) I see this as a leak in prototype because using the attachEvent (button 2 in the test page) equivalent does not present the same leak. Attaching the element into the document (button 3 in the test page) also does not present the same leak. I have found that the fix from this thread -http://groups.google.com/group/prototype-core/browse_thread/thread/c2... solves the problem, contrary to the original poster who reported it only for IE6, my test program shows that the leak happens for IE7 as well. For my project, instead of using the straight forward: parent.replaceChild(newnode, oldnode); I have to do this instead: $(oldnode).select(*).each(function(e) { $(e).stopObserving();}); parent.replaceChild(newnode, oldnode); That's quite a lot of work. Hope to see this fixed in 1.6.1 if not 1.6.0. Cheers, Phuah Yee Keat testobserve.html 2KViewDownload --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Looks like Firefox already has it: https://developer.mozilla.org/En/DOM_Events Googling DOMNodeRemovedFromDocument site:microsoft.com brings no joy, however. :-) -- T.J. Crowder tj / crowder software / com On Oct 30, 9:02 am, T.J. Crowder [EMAIL PROTECTED] wrote: Hi, FWIW, I don't think this is a bug in Prototype. Prototype keeps track of the event handlers it hooks up so it can unhook them on page unload to work around memory leaks in IE. (This also lets it provide the new stopObserving functionality in release 1.6 where you don't have to specify the handler to unhook things.) This means that it has to use a small bit of memory to track the handlers. That's not a *leak*, I wouldn't say, but a *use* of memory. If you remove an element from the DOM (directly or by removing its ancestor), I think it's incumbent on your application logic to remove any handlers you've registered for it as well. Your function that does this with a select(*) is obviously one way to do that, but ideally there'd be something more finely-grained that app logic could handle. Personally, I wouldn't want to see the select(*) thing added to Prototype (for instance, in the Element.update method), because I don't want that overhead every time I remove an element if I know I don't have any handlers registered on it. Someday we may get access to the DOM event DOMNodeRemovedFromDocument[1]. Now that would be nice, because it would give us notification of an element being removed and let us release any memory we have associated with it. (I'm not sure I'd want Prototype to do it for me even then, but I'll make that call when/if we get the event.) [1]http://www.w3.org/TR/2000/REC-DOM-Level-2-Events-20001113/ If it's very complicated in your app to know which elements within the element being replaced are being watched, you might look at event delegation instead, which involves fewer handlers placed at a higher (e.g., container) level. FWIW, -- T.J. Crowder tj / crowder software / com On Oct 30, 7:57 am, Yee Keat Phuah [EMAIL PROTECTED] wrote: Hi, Was trying to cut the memory leak reported with this tool -http://home.wanadoo.nl/jsrosman/. Found out that there are some memory leaks in prototype's Event.observe, only when the element that is observed is not within the document DOM, this might happen when 1. I have not attached the element into the DOM, or that 2. I have removeChild or replaceChild the element that was observed on. (this is what I encountered in my project) I see this as a leak in prototype because using the attachEvent (button 2 in the test page) equivalent does not present the same leak. Attaching the element into the document (button 3 in the test page) also does not present the same leak. I have found that the fix from this thread -http://groups.google.com/group/prototype-core/browse_thread/thread/c2... solves the problem, contrary to the original poster who reported it only for IE6, my test program shows that the leak happens for IE7 as well. For my project, instead of using the straight forward: parent.replaceChild(newnode, oldnode); I have to do this instead: $(oldnode).select(*).each(function(e) { $(e).stopObserving();}); parent.replaceChild(newnode, oldnode); That's quite a lot of work. Hope to see this fixed in 1.6.1 if not 1.6.0. Cheers, Phuah Yee Keat testobserve.html 2KViewDownload --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Hi, On Thu, Oct 30, 2008 at 5:02 PM, T.J. Crowder [EMAIL PROTECTED] wrote: FWIW, I don't think this is a bug in Prototype. Prototype keeps track of the event handlers it hooks up so it can unhook them on page unload to work around memory leaks in IE. (This also lets it provide the new stopObserving functionality in release 1.6 where you don't have to specify the handler to unhook things.) This means that it has to use a small bit of memory to track the handlers. That's not a *leak*, I wouldn't say, but a *use* of memory. As noted from the thread I quoted above, prototype 1.6 does not unhook them on page unload anymore, it just do cache[id][eventName] = null. prototype 1.5 does unblock them on page unload using stopObserving while iterating over each of the handlers. Cheers, Phuah Yee Keat --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
Hi, As noted from the thread I quoted above, prototype 1.6 does not unhook them on page unload anymore, it just do cache[id][eventName] = null. I'll be interested in hearing what someone from Core has to say about that change; I thought maybe they'd realized that that was all that was needed to break the chain (since the issue with IE relates to *circular* references, not just references), but I never quite understood how doing that would be breaking them. But now I can't get IE6 to leak memory if I unload the page even using Crockford's queuetest2[1] (which is designed to demonstrate the circular reference leak, and does so, but within an active page). Maybe somewhere along the line M$ managed to slip a fix into a service pack that at least made IE semi-reliably clean up on page *unload*, even though not otherwise. But I know it doesn't do it reliably. [1] http://javascript.crockford.com/memory/queuetest2.html All of that is tangental, though, to the main point of your thread: Cleaning things up when the page is still loaded, and you're just removing elements. I still think doing that is in the domain of the application logic, not the library. That's my take; others may differ. :-) -- T.J. Crowder tj / crowder software / com On Oct 30, 9:41 am, Yee Keat Phuah [EMAIL PROTECTED] wrote: Hi, On Thu, Oct 30, 2008 at 5:02 PM, T.J. Crowder [EMAIL PROTECTED] wrote: FWIW, I don't think this is a bug in Prototype. Prototype keeps track of the event handlers it hooks up so it can unhook them on page unload to work around memory leaks in IE. (This also lets it provide the new stopObserving functionality in release 1.6 where you don't have to specify the handler to unhook things.) This means that it has to use a small bit of memory to track the handlers. That's not a *leak*, I wouldn't say, but a *use* of memory. As noted from the thread I quoted above, prototype 1.6 does not unhook them on page unload anymore, it just do cache[id][eventName] = null. prototype 1.5 does unblock them on page unload using stopObserving while iterating over each of the handlers. Cheers, Phuah Yee Keat --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
On Oct 30, 11:07 am, T.J. Crowder [EMAIL PROTECTED] wrote: Hi, As noted from the thread I quoted above, prototype 1.6 does not unhook them on page unload anymore, it just do cache[id][eventName] = null. I'll be interested in hearing what someone from Core has to say about that change; I thought maybe they'd realized that that was all that The change was pulled off (perhaps it was considered too obtrusive or there was no time to test it thoroughly). I think we should at least provide some kind of `purgeObservers` method and educate people about what happens when you update document and leave observers hanging in memory. [snip] -- kangax --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory Leak on IE
The change was pulled off (perhaps it was considered too obtrusive or there was no time to test it thoroughly). I was talking about the (apparent) change between 1.5 and 1.6. I think we should at least provide some kind of `purgeObservers` method Yeah, something that accepts an element and clears observers from it and all of its descendants, like the OP's select(*) or ideally something more efficient using the cache internal to prototype. Then in situations where you know some things are hooked, you can do: $('container').purgeObservers().update(newcontent); -- T.J. Crowder tj / crowder software / com On Oct 30, 4:53 pm, kangax [EMAIL PROTECTED] wrote: On Oct 30, 11:07 am, T.J. Crowder [EMAIL PROTECTED] wrote: Hi, As noted from the thread I quoted above, prototype 1.6 does not unhook them on page unload anymore, it just do cache[id][eventName] = null. I'll be interested in hearing what someone from Core has to say about that change; I thought maybe they'd realized that that was all that The change was pulled off (perhaps it was considered too obtrusive or there was no time to test it thoroughly). I think we should at least provide some kind of `purgeObservers` method and educate people about what happens when you update document and leave observers hanging in memory. [snip] -- kangax --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
I'm not sure what you mean exactly. If you mean that the patched version 1.5.1 looks like a working fix, I would agree. However, on the original tests I sent you, I don't see a memory increase on refresh, but on closing the window none of that reserved memory is released. If you were referring to that 20MB difference between starting the testing and closing all sets of windows? Since I didn't see it changing, I admit to rather ignoring it on the assumption that it was 'normal' - I haven't memory profiled IE much, but I the few times I recall looking I certainly don't recall seeing less than 50 MB. Five minutes of testing simple sites shows IE spiking quickly to 44MB and just hanging out within 10-15MB of there. So that doesn't concern me much. Thanks, Geoff Granum On 5/5/08, John-David Dalton [EMAIL PROTECTED] wrote: The fact that the memory doesn't stair step tells me that there is probably not a leak and what you are experiencing is just the normal amount of memory IE is reserving for each new page instance. If however, you refresh and memory usage keeps growing then we know its a memory leak. What are others thoughts? - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
On Begin Test: 22MB prototype_1_5_1_1 ( as available for download ) Ten 10MiB Windows Open: 109MB Ten windows after all closed: 105MB prototype_1_5_1_1 ( with patch ): Ten 10MiB Windows Open: 109MB Ten windows after all closed: 48MB Same main window, once more: Ten 10MiB Windows Open: 109MB Ten windows after all closed: 41MB Same main window, once more: Ten 10MiB Windows Open: 110MB Ten windows after all closed: 61MB Same main window, once more: Ten 10MiB Windows Open: 112MB Ten windows after all closed: 45MB _prototype_1_5_1_1 ( 1.6.0.2+): Ten 10MiB Windows Open: 157MB Ten windows after all closed: 109MB Refreshing the window doesn't increase the memory for me, either. I would guess this has more to do with how IE refreshes ( e.g. not very deeply ). That is a hypothesis, not a statement; I have no idea how IE deeply IE refreshes. It looks like the 1.5 with patch is good; the fluctuations seem to have more to do with IE not being aggressive with the clean up. Watching the memory decrease on each window close, sometimes it would drop 4MB, sometimes 12MB, and all numbers in between. If Drip is seeing a real leak it is at least a small leak; not something that refers back to the window object itself, causing NOTHING to be unloaded. Ah, IE. Can I tell you how glad I am Sun won that MS J++ lawsuit? If there were a JVM with a GC this bad I'd... I'd take up landscaping or something. Having NO GC is better than a GC that uses a random number generator to collect objects. The 1.6 version is collecting more than 1.5 was before the patch, but there is still obviously something hanging out. I wonder if the garbage collection you do in prototype is cleaning up all (well, _nearly_ all) the objects created on init, but all the primary objects that live in window scope are still living in memory because IE thinks the window still exists? I don't know how many objects are created after the script itself is done loading, so I'm pretty much in the dark here. Hmm. And perhaps on a refresh, IE knows it can delete the window? Or just rebuild the structure on the same window object it was using previously, thus replacing the objects, not adding more. I didn't have all that great of luck with Drip when I tried it on our full app. Sometimes it showed hundreds of leaks, sometimes 4 or five, and those would be like a frame object pointing to a blank.htm file. Always for the same action. I don't take it personally. Drip is at what, version 0.5? Impressive stuff. Thanks again, - Geoff Granum On May 3, 6:37 pm, John-David Dalton [EMAIL PROTECTED] wrote: Thanks for the tests Geoff, I have uploaded them here:http://protolific.net/memory_leak_testcase.zip I patched the Prototyep 1.5.1 file. There is a second file named _prototype_1_5_1.js which is my latest git version compiles (so it is really 1.6.0.2+ with the patch in it is as well. I still get the memory leak with both patched versions. I think it has more to do with the number of nested frames than it does with String#escapeHTML. When I close the child window the memory usage drops down (I am using the windows task manager and looking at the IE process). When I refresh the child window the memory usage does not increase (this shows that maybe its not a leak). However, Drip suggests the the memery is never released.Wwhen refreshing the child window in drip it stair steps the memory usage. This may just be a Drip bug. In your 1 MB leak example you load Prototype js 4 times - 1000kb/4 = 250kb per page instance.(you have a body page without Prototype as well that is propbably using up some of that memory as well) Can you try the tests that I have linked to and confirm that the memory spike is still an issue with the patches applied (maybe check the patch to see if it's applied write). - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
Thanks for the tests Geoff, I have uploaded them here: http://protolific.net/memory_leak_testcase.zip I patched the Prototyep 1.5.1 file. There is a second file named _prototype_1_5_1.js which is my latest git version compiles (so it is really 1.6.0.2+ with the patch in it is as well. I still get the memory leak with both patched versions. I think it has more to do with the number of nested frames than it does with String#escapeHTML. When I close the child window the memory usage drops down (I am using the windows task manager and looking at the IE process). When I refresh the child window the memory usage does not increase (this shows that maybe its not a leak). However, Drip suggests the the memery is never released.Wwhen refreshing the child window in drip it stair steps the memory usage. This may just be a Drip bug. In your 1 MB leak example you load Prototype js 4 times - 1000kb/4 = 250kb per page instance.(you have a body page without Prototype as well that is propbably using up some of that memory as well) Can you try the tests that I have linked to and confirm that the memory spike is still an issue with the patches applied (maybe check the patch to see if it's applied write). - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
Patch addresses the memory leak: http://github.com/jdalton/prototype/commit/0353246ac2ad439260905799798f0069d9a7d0ca Patch addresses other issues with escapeHTML and unescapeHTML: http://github.com/jdalton/prototype/commit/6010a300c39b0c66394706e9edb8b110b5932d9e Unit tests for the patch: http://github.com/jdalton/prototype/commit/1fcf26a2810da2e8fee7323e1613dfad094181f8 I think there may be other places where we attach elements, I will see if I can duplicate this over the weekend. - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
Ah, I missed the hold on to all references on page unload part : ) John, wouldn't it make sense to null the references in purgeListeners function (since it's already attached to unload)? - kangax On May 2, 6:30 pm, John-David Dalton [EMAIL PROTECTED] wrote: Patch addresses the memory leak:http://github.com/jdalton/prototype/commit/0353246ac2ad43926090579979... Patch addresses other issues with escapeHTML and unescapeHTML:http://github.com/jdalton/prototype/commit/6010a300c39b0c66394706e9ed... Unit tests for the patch:http://github.com/jdalton/prototype/commit/1fcf26a2810da2e8fee7323e16... I think there may be other places where we attach elements, I will see if I can duplicate this over the weekend. - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Memory leak in IE; prototype version 1.5.1 and up. Caused by String.prototype.escapeHTML improvements
I was wondering if some of the code I saw in the upcoming 1.6.0.3 milestone might have addressed this. I should have mentioned that, apologies. The tentative fix I outlined above doesn't seem to leak; the original leak was *very* obvious due to the number of nested frames in the child window that were using prototype. As in, ten or more MB per window open/close, every time. I'm out the door in a moment here, so I can't write a new testcase right now but I'll outline and try to implement it tomorrow: a.) create an application-like window that uses prototype - ours used a (nested) frameset to hold the navigation bar, button bar, body and footer (yes, we ARE moving away from frames, why?). b.) a button on the button bar uses an onclick event to manually open a window. Bonus points if you store the new window in a manager and delete it after the window closes ( this was done to bring the window to focus on a second click of the button ). c.) The child window will have frames. To make the leak really obvious, nest some frames and have them all use a large JS file import. d.) Open and close the child window in IE, taking note of the memory usage in Windows Task Manager. Profiling tools such as Drip miss this _in our application_. Since we use ( a large number of ) frames, it's hard to say if it misses the leak, or is breaking on the frames. What I see after the above fix ( which I get the impression would not be wise to release?) is a spike in memory from about 80MB on the full load of the web app, to about 150 peak, with the usage bouncing around 100/110 and spiking on many fast page changes. After about ten seconds idle IE will free up 10 or so MB. After another 20 seconds it will free up the rest, dropping back to the fresh load state. Before the above fix every child window open/close was about 12MB. Sitting idle did nothing to drop memory usage. Navigating entirely away (e.g. google, about:blank ) didn't even flush the memory. 500MB, 750MB. It was pretty easy to ramp it up. On a lesser machine I'm sure it would be incredibly painful. Having 4GB of RAM I managed to miss the leak entirely for quite a while. Thank you both very much for the incredible response. That you have already looked at the code in question and tried to create a testcase is bloody impressive. I will see about getting you a repro tomorrow. On May 2, 3:30 pm, John-David Dalton [EMAIL PROTECTED] wrote: Patch addresses the memory leak:http://github.com/jdalton/prototype/commit/0353246ac2ad43926090579979... Patch addresses other issues with escapeHTML and unescapeHTML:http://github.com/jdalton/prototype/commit/6010a300c39b0c66394706e9ed... Unit tests for the patch:http://github.com/jdalton/prototype/commit/1fcf26a2810da2e8fee7323e16... I think there may be other places where we attach elements, I will see if I can duplicate this over the weekend. - JDD --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Prototype: Core group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---