[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 -~--~~~~--~~--~--~---
