Closed Bug 84136 Opened 23 years ago Closed 23 years ago

Closing windows does not free all memory they have allocated

Categories

(Core :: DOM: HTML Parser, defect)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: ezh, Assigned: harishd)

References

Details

(4 keywords, Whiteboard: pdt+)

Attachments

(7 files)

OK I started the thred "mem usage" in mozilla's performance newsgroup and we had such a test: Blank page: 17 484K Go to bluesnews.com: 21 492K Open the news archive in a new window: 24 356K +2 864K close the news archive window: 23 924K - 432K open a random link in a new window: 24 572K + 648K close that window: 24 512K - 60K open random link in new window: 25 364K + 852K close that window: 25 180K - 184K It appears, with every new window, to be adding ram on, and when you close that window, it frees up a small percentage of the ram used. I can understand it eating up a little ram the first few times, as it's recording the recent history and after a while it should taper off, but those numbers seem rather large. Tested at 2001053004
Changing OS to All since I see this on win98 and the test was done on w2k.
OS: Windows 98 → All
Summary: closeing window does not freeing all memory they have allocated → closeing windows does not freeing all memory they have allocated
Added perf, upped severity. Who should be handling this bug? Adding waterson to cc.
Severity: normal → major
Keywords: perf
Adding footprint keyword since this is a memory use issue.
Keywords: footprint
This could be any number of things, but is most likely heap fragmentation due to the allocator. If someone can demonstrate that there is really a leak here, then we should change the bug to mean ``fix that leak''. Otherwise, I'm not really sure what we can do about this..
This test should be redone with the Lea allocator (as per my messages in performance). 600-800K of fragmentation loss per window opened is rather large....
I'll try to do a lea build and post the results here later today.
If you're going to compile with lea, be careful of bug 73053 and bug 72497. Perhaps we should add a dependency, though I'm not certain that Lea is the only issue here (probably not, in fact).
If the original test was done with Win2K then using lea is not really a solution for that platform (since we can only use lea on *nix right?). Though that would be no reason _not_ to use lea where avalible of corse. Don't we have tools for checking if this really is a fragmentation loss only or if we infact hold more memory allocated?
We certainly have tools to determine if it's a leak. Get a debug build, set XPCOM_MEM_BLOAT_LOG=<some-file>, run, and look at the contents of <some-file>.
I run mozilla on a PII 266 with 64 MB and this is a major problem for me Everything is fine when I start using mozilla but after 1 or 2 hrs of intense browsing (with several windows opened) mozilla takes 60-70 MB and the machine slows down to the point where I have to quit mozilla... and return to IE (no prb with it) just a thought: On NT (I think even on Win9x) there are heaps (functions like HeapCreate() HeapAlloc() HeapDestroy() and HeapCompact() to give the memory back to the OS) I searched through LXR and I did not find any use of those functions It's strange because it is one of the nice NT feature and one of the key to have good multithread/multiprocessor performance because there's less contention with several heaps. It would be nice if all memory specific to one window could be allocated in such a heap. That way the heap could be destroyed when the window is closed leading to less fragmentation (if no leak, but I can't belive there can be so much memory leaked :-)) in the 'main' heap used by malloc() Always looking at the code I see many references to 'arenas' (don't know what it is) I don't know how they relate to 'heaps'
reseting os. Please don't change from Windows X to All unless you can reproduce on something that isn't Windows.
OS: All → Windows 98
resetting OS to all. Just ran a test (FreeBSD 4.1 Mozilla 20010605xx debug build). Start, go to mozilla.org. Open new window (blank), click on mozilla.org, close. Repeat a dozen times. Average memory loss is ~1.5-2MB per window open, ~300K per click on mozilla.org (measured by watching Size in top). Memory usage never goes down. Note: this is the system (BSD) allocator, that tends to allocate a lot of pages and not give them back. However, BSD usually stabilizes and stops using more. This has not stabilized, both RES and Size increase for as long as I tried it.
OS: Windows 98 → All
BTW, Chris, this does NOT seem to just be heap fragmentation, or the lossage would decline, not stay at a fairly steady amount over a loss of 50MB. I just redid my test without browsing to mozilla.org (just opening/closing blank windows). Same loss as best I can tell, or very close. Lost 50MB without any slowdown (~1.4-1.5MB per open/close), and RES tracked the increase in Size (this is a 512MB machine).
Bloat logs show lots of XUL Elements/attributes lost, and 2.6 million nsStr's. The amount reported lost (13MB) is WAY lower than the increase in size seen (~40MB that run).
Things like nsStr and nsTextNode point to text buffers that aren't counted in the bloat logging (which is a rather blunt tool and easy to misinterpret). I've been seeing similar content leaks.
Keywords: mlk
The leaked GlobalWindowImpl and nsDocument look more like roots than anything else. I wonder if the patch on bug 81289 helps at all. (It fixes at least one GlobalWindowImpl leak, but I don't *think* it could fix a GlobalWindowImpl + nsDocument leak.)
_Nice_! Ok, so can we figure out how to make _one_ nsDocument leak? (These leaks clearly don't appear on tinderbox, so they must be the result of some user interaction.) For example, - Do we leak when closing a single window using ``File->Exit''? If not... - Do we leak when opening a second window, then using ``file->exit'' to shut down the whole app? If not... - Do we leak when opening a second window, then closing it using ``file->close'', then shutting down the app? etc. This is the game that dbaron and others are ever so familiar with. (At one point, just _mousing over_ the URL bar would cause the entire window to leak.) To the extent that we can minimize the interaction required to kill the leak, we win... thanks for your help, Randell!
I've applied dbaron's patch and am rebuilding. Once it's done, I'll try. In particular, when sequence I used before was: Open mozilla (blank). ^n (new window, blank). Mouse to close box, click. Note: sidebar was open, not displaying anything interesting. NOTE: I was getting these every so often when opening/closing: ###!!! ASSERTION: root element not in document: 'doc != nsnull', file nsXULContentBuilder.cpp, line 1523 I do not know if this was directly related to leaks; I got rather more of these than i opened windows which was why I didn't jump on it. AhA! looking at it more closely, on each open-window, I got one of those assertions for each window I had perviously opened! Hmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm Also, as I opened each window I got this: nsWidget::~nsWidget() of toplevel: 278 widgets still exist. WEBSHELL+ = 98 WEBSHELL+ = 99 Enabling Quirk StyleSheet WEBSHELL+ = 100 WEBSHELL+ = 101 Enabling Quirk StyleSheet Enabling Quirk StyleSheet With the webshell numbers going up and up...
Hrm. I haven't seen leaked webshells for a long time (at least not when paying attention).
I do have a few mods in my tree to make nsIViewManager *'s into nsCOMPtr<nsIViewManager>'s, and test for null viewmanagers. I should also note I'm trying to track a problem with View's being freed and reallocated (80203). I can provide a cvs diff of any modified files if anyone thinks my mods could be the cause. BTW, dbaron's patch didn't seem to help. I'm backtracing the assertion right now.
Please do provide a diff of your modified files, although that doesn't *seem* like it should be the problem since the view manager only has weak raw pointer back to its observer.
BTW, the asserts are due to (deep within) this: nsWindowMediator::UnregisterWindow() I'll attach a GDB session with details. I'll also grab a diff
I thought we got rid of the parser / content sink / document circular references long ago, but apparently we haven't. When I start mozilla, open a new window with ^N, and close the two windows with the X in the corner, I see 3 nsDocument leaked. The first two are XBL-related, and the third is the real one. This nsDocument (#24 to me) has one outstanding reference, from an nsXMLContentSink. The nsXMLContentSink has two outstanding references, one from an nsParser and one from an nsScriptLoader. The nsParser has two outstanding references, one from an nsXMLDocument and one from an nsXMLContentSink. So the ownership model that's leaking looks something like this: <-------------- --> nsParser --------------> nsXMLContentSink <----- nsScriptLoader / / | / \ / -------------- nsDocument <- | V everything else (I assume) I'm guessing this is all because some member function of the parser isn't called.
Assignee: asa → harishd
Component: Browser-General → Parser
QA Contact: doronr → bsharma
Thank you dbaron! Adding moz 0.9.2 to keywords; hopefully this is straightforward to fix - it's a massive mem leak.
Keywords: mozilla0.9.2
FYI: While using the "multidesk" utility for windows (from techsuperior.com) I can get already closed windows to come back as dead frames when I switch the desktops.
Summary: closeing windows does not freeing all memory they have allocated → Closing windows does not freeing all memory they have allocated
(if you're going to be picky about grammar... :) )
Summary: Closing windows does not freeing all memory they have allocated → Closing windows does not free all memory they have allocated
here is my take with mozilla 2001052404 on windows 2000 (512Mb ram) This is from task manager Preperation: * Open browser. * set home page to blank page * clear disk and memory cache * close mozilla test run Task Mem usage (kb) VM size (kb) Open Mozilla 17.516 11.356 Minimize Mozilla 1.084 11.436 Restore Mozilla 6.188 1176 (why so little) www.slashdot.org 15.340 13.828 (less than after startup) open banner in nw (linux.com) 19.740 17.872 Open Mail 25.192 22.312 Close mail 24.992 22.100 Close linux.com 24.232 21.344 ctrl-n & www.theserverside.com 24.908 21.948 Minimize www.theserverside.com 7.512 21.960 minimize www.slashdot.org 3.976 21.960 restore www.slashdot.org 11.108 21.960 restore www.theserverside.com 13.200 21.960 Open mail again 22.104 23.304 minimize mail 11.312 23.316 minimize all 3.072 24.316 A few interesting things in this memory usage 1. Mozilla seems to free the memory when minimizing windows. 2. Start-up, minimize and restore result in a LOWER memory usage than just start-up 3. The VM size does not decrease when mozilla frees memory. dunno if that is a windows thing.
FWIW, Win32 moves most of an application's pages to the standby list when the application windows are minimized. This reduces the resident set size as reported by the task manager (N.B., the pages aren't actually flushed to disk until they get evicted from the standby list). So, this is not something magic that mozilla is doing. But, this really isn't all that important, because dbaron has already isolated what the problem is.
From the cvs log of nsXULDocument.cpp: revision 1.49 date: 1999/04/06 07:48:21; author: waterson%netscape.com; state: Exp; lines: +7 -9 Fixed a _big_ memory leak: circular reference between the document, content sink, and parser. Now the document releases the parser immediately after telling it to start parsing. I guess the more recent stuff was with HTML. It would make sense to me if the content sink didn't have an owning pointer to the document. This would also reduce the risk of leaking a document from leaks in parser or network code. It would mean that one would have to stop the content sink if the document was going away, but that would seem like a reasonable thing to do anyway (and it should have happened earlier anyway). Although maybe breaking the cycle some other way would be easier...
Harish is on vacation till Thursday. Heikki has agreed to take a look at this. Thanks, Heikki!
The patch I just attached removes the strong reference to the parser in the XML document. Currently there just isn't any need for it. The approach is almost 100% copy of the way XUL document does it. This approach will not work in HTML (at least not as simply) since the HTML document is actually using the parser for various things. I am now looking at how to break the reference to the document from the content sink.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
Any further action on HTTP? Can someone r/sr/a the partial (XML) patch that Heikki attached? This one is a 0.9.2 target, and a massive leak.
Heikki's approach is correct but it doesn't seem to fix the leak. I've started working on it.
Marking pdt+ in the status whiteboard. This means that this has been approved by the PDT as a rtm stopper.
Whiteboard: pdt+
FYI: I see the leak only when I use ctrl-n to open up a new browser window.
I use CTRL+Click on a link to open the link in new window.
FYI: The purify run was done without heikki's patch.
The root of the problem, I think, seems to be in nsXBLService::FetchBindingDocument(). CCing hyatt.
The above comment should read: " Avoid getting into document-parser-sink circularity by delaying StartDocumentLoad "
Attached file nsXBLService::FetchBindingDocument) (deleted) —
sr=hyatt
a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)
Blocks: 83989
Fix is in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
*** Bug 86561 has been marked as a duplicate of this bug. ***
In which build can I see the improvement? with 2001062004 is no difference to 2001061704. Actually at the startup with rus.delfi.ee as a homepage it taked 2.5Mb mem more (total 23.5) than the previous biuld I used... Can anyone send a log for builds with this fix to compare with previous one?
The leak only occurred when opening/closing windows (watch SIZE (VM size) while opening (with ^N) and closing (with close X in corner) windows. We'd leak about 1.4MB per open/close.
I'm talking about it, actually... I open moz with rus.delfi.ee as my homepage (~23.5Mb) Open 5 new windows with CTRL-N (~42Mb) Close 5 windows (~30Mb). So this was about the same without this patch... build 2001062204
Eugene: The patch does fix the document-parser-sink leak. I'm not sure why you don't see any difference.
Well, you opened/closed 5 windows, and your memory usage went from 23.5MB to 30MB. That sounds like about 1.5MB/window opened/closed, but 5 windows isn't enough to be sure either way using tools like ps and top. Note that measuring mozilla memory usage can be tricky, and the OS can affect things as well. I opened ~26 new windows (all displaying about:blank) and closed them (one at a time), and lost about 50MB of memory. The leak was very much real.
Can you provide mem usage with ru.delfi.ee as a homepage, please?
Sorry, I may not be able to - I leave for vacation in a few hours and am pretty busy in the meantime. Perhaps someone else can?
A did some tests. Hardware: Duron 950, 384Mb RAM, IDE RAID-0 with 2*30Gb IBM DTLA HDD Software: Win98 SE, Mozilla 2001062204, Opera 5.11, TaskInfo 2000 3.0.6.11 (beta) (tried to test NN 4.77, but every time it crashed itself or crashed Windows :)) How I tested: 1. Opened the program with rus.delfi.ee as a homepage 2. Looked how much memory it used (inMem Kb) 3. Pressed CTRL-N 25 times (so total 26 windows were opened) 4. Looked at CPU usage 5. Looked how much time taked untill the last window showed "Done." (with wallclock) 6. Looked mem usage 7. Closed 25 windows with "X" 8. Tried to measure how much time it tooked 9. Looked how much mem is used after all is complete. Results: Mozilla: 2. 21200 Kb 4. ~95% CPU were used (!!!) 5. 3 min 16 sec (!!!) 6. 115328 Kb (!!!) 8. 12 sec (!!!) 9. 38787 Kb Opera: NB! I was not able to get any times since Opera does not load the homepage in new window (the gloab and window homepage was set to rus.delfi.ee). So I opened 25 new blank windows (they appeared/closed without any delay as soon as I pressed CTRL-N or "X") and pressed for eachone CTRL-Space (loaded the homepage). So I've skipping p. 4, 5, 8 2. 6256 Kb 6. 20512 Kb 9. 8988 Kb Comments?
The times and amount of memory used can be laid at the foot of XUL partially, and the rest at overall structural issues in Mozilla. Something like this would make an interesting quantify case to check. The fact that after opening/closing 25 windows memory usage was 38MB (down from over 110MB) means that the major leak is definitely gone. (In the past, closing windows brought almost no memory back.)
I found another (major?) leak involving opening and closing new windows: bug 87466. Fixing this reduces the amounts of memory that we *never* free to acceptably low numbers (even for me). However, even with that patch, if I start mozilla (debug), I see 42MB memory usage, and if I open 6 more windows I see memory usage go up to 52MB, and it doesn't come back down when I close those Windows. However, according to our leak detection tools we are freeing the memory before we exit. So what I'm seeing is probably the hardest type of leak to diagnose -- one where we free memory later than we should have, but do free it. (It's hard for a leak detection tool to know when something *should* have been freed and detect that it was freed later than it should have been. Many objects are supposed to be very short-lived and some are supposed to exist for the entire lifetime of the program.) Anyway, I'll look into it a bit more...
I filed bug 87472 on the general issue I mentioned in my previous comment (which is I guess what this bug was originally filed for).
Never mind -- see why I marked bug 87472 invalid.
Verified on: build: 2001-07-02-04-Trunk platform: WinNT Marking verified as per above developer comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: