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)
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
Reporter | ||
Comment 1•23 years ago
|
||
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
Comment 2•23 years ago
|
||
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
Comment 4•23 years ago
|
||
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..
Comment 5•23 years ago
|
||
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....
Comment 7•23 years ago
|
||
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?
Comment 9•23 years ago
|
||
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>.
Comment 10•23 years ago
|
||
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'
Comment 11•23 years ago
|
||
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
Comment 12•23 years ago
|
||
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
Comment 13•23 years ago
|
||
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).
Comment 14•23 years ago
|
||
Comment 15•23 years ago
|
||
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).
Comment 16•23 years ago
|
||
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
Comment 17•23 years ago
|
||
Comment 18•23 years ago
|
||
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.)
Comment 19•23 years ago
|
||
_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!
Comment 20•23 years ago
|
||
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...
Comment 21•23 years ago
|
||
Hrm. I haven't seen leaked webshells for a long time (at least not when paying
attention).
Comment 22•23 years ago
|
||
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.
Comment 23•23 years ago
|
||
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.
Comment 24•23 years ago
|
||
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
Comment 25•23 years ago
|
||
Comment 26•23 years ago
|
||
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
Comment 27•23 years ago
|
||
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
Comment 28•23 years ago
|
||
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.
Updated•23 years ago
|
Summary: closeing windows does not freeing all memory they have allocated → Closing windows does not freeing all memory they have allocated
Comment 29•23 years ago
|
||
(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
Comment 30•23 years ago
|
||
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.
Comment 31•23 years ago
|
||
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.
Comment 32•23 years ago
|
||
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...
Comment 33•23 years ago
|
||
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.
Comment 36•23 years ago
|
||
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.
Assignee | ||
Comment 37•23 years ago
|
||
Heikki's approach is correct but it doesn't seem to fix the leak. I've started
working on it.
Comment 38•23 years ago
|
||
Marking pdt+ in the status whiteboard. This means that this has been approved
by the PDT as a rtm stopper.
Whiteboard: pdt+
Assignee | ||
Comment 39•23 years ago
|
||
FYI: I see the leak only when I use ctrl-n to open up a new browser window.
Reporter | ||
Comment 40•23 years ago
|
||
I use CTRL+Click on a link to open the link in new window.
Assignee | ||
Comment 41•23 years ago
|
||
Assignee | ||
Comment 42•23 years ago
|
||
FYI: The purify run was done without heikki's patch.
Assignee | ||
Comment 43•23 years ago
|
||
The root of the problem, I think, seems to be in
nsXBLService::FetchBindingDocument(). CCing hyatt.
Assignee | ||
Comment 44•23 years ago
|
||
Assignee | ||
Comment 45•23 years ago
|
||
The above comment should read:
" Avoid getting into document-parser-sink circularity by delaying
StartDocumentLoad "
Assignee | ||
Comment 46•23 years ago
|
||
Comment 47•23 years ago
|
||
sr=hyatt
r=heikki, nice going.
Comment 49•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
Assignee | ||
Comment 50•23 years ago
|
||
Fix is in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 51•23 years ago
|
||
*** Bug 86561 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 52•23 years ago
|
||
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?
Comment 53•23 years ago
|
||
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.
Reporter | ||
Comment 54•23 years ago
|
||
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
Assignee | ||
Comment 55•23 years ago
|
||
Eugene: The patch does fix the document-parser-sink leak. I'm not sure why you
don't see any difference.
Comment 56•23 years ago
|
||
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.
Reporter | ||
Comment 57•23 years ago
|
||
Can you provide mem usage with ru.delfi.ee as a homepage, please?
Comment 58•23 years ago
|
||
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?
Reporter | ||
Comment 59•23 years ago
|
||
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?
Comment 60•23 years ago
|
||
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.)
Comment 61•23 years ago
|
||
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...
Comment 62•23 years ago
|
||
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).
Comment 63•23 years ago
|
||
Never mind -- see why I marked bug 87472 invalid.
Comment 64•23 years ago
|
||
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.
Description
•