39742 - File | Quit takes a huge amount of time (exit or shutdown slow)

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect, P3)

Description

[Marko Macek]

File|Quit takes a huge amount of time (it get's exponentially slower as the
number of opened windows increases).

Related to bug 28639 and probably bug 30660.

Eric Krock asked me to file a separate bug because there might be a more
efficient way to implement File|Quit.

Comment 1

[Jeremy M. Dolan]

each window seems to take about 1 second here. took 8 seconds to file|quit when
i had 7 windows open. may 11th linux build, p2/266, 96M.

Comment 2

[Marko Macek]

7 windows is not much. Try 50 or so.

Comment 3

[Marko Macek]

Note: make sure your machine doesn't swap while you test this. The is not due to 
lack of RAM.

Comment 4

[Stephen Ostermiller]

You must have an amazing machine to be able to try to duplicate this bug without
swapping.  After opening 26 windows, ps reports that mozilla is consuming 66% of
my memory and 27% of my cpu time.  (Any more and I'd be swapping like crazy.)
Each copy is sitting there on about:blank.  My machine isn't too shabby, 400 MH,
128MB.  I'm using the default skin with my bookmarks in the sidebar.  Maybe a
simpler skin would help?  

Anyway, under this configuration using, File->Quit took 4 minutes and 38
seconds.  The windows definitly started to disappear noticably faster once the
number of windows left open was under 10.

Comment 5

[Marko Macek]

With 50 windows open I had about 11MB swap used. There was very little disk IO
being performed while mozilla was exiting. And I have 128MB ram/433 Celeron. CPU
usage by mozilla was ~99% during the exit procedure. When it hits 6-7 windows it
just quits immediatelly (this could be a bug...) Total time for 50 windows was
about 9-10 minutes.

Comment 6

[battery841]

I have noticed this problems for a while.  I know after prolonged use (20
minutes or more) and I try to quit, my hard drive will be accsesing for about 30
seconds to a minute.  I'm going to change this to new.  It seems like theres
enough evidence for this bug.

Comment 7

[Asa Dotzler [:asa]]

Sorry for the spam.  New QA Contact for Browser General.  Thanks for your help
Joseph (good luck with the new job) and welcome aboard Doron Rosenberg

Comment 8

[Asa Dotzler [:asa]]

hmm, not sure who should be watching this bug.  any ideas?

Comment 9

[Marko Macek]

I'd make it depend on bug 42321 or something like that.

Comment 10

[Asa Dotzler [:asa]]

42321 definitely looks like a piece of the problem.  setting this bug to depend
on bug 42321

Comment 11

[Asa Dotzler [:asa]]

this may also depend on bug 44535.  setting dependency.

Comment 12

[Asa Dotzler [:asa]]

OK< I got this to crash. Reloading the main page about three times produced a
crash with a talkback report. Stack trace from Talkback below.  reassigning and
setting default owner.


   ImageConsumer::OnDataAvailable

[d:\builds\seamonkey\mozilla\gfx\src\nsImageNetContextAsync.cpp, line 370]

   nsMultiMixedConv::SendData

[d:\builds\seamonkey\mozilla\netwerk\streamconv\converters\nsMultiMixedConv.cpp,
line 434]

   nsMultiMixedConv::OnDataAvailable

[d:\builds\seamonkey\mozilla\netwerk\streamconv\converters\nsMultiMixedConv.cpp,
line 199]

   nsDocumentOpenInfo::OnDataAvailable

[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 252]

   nsHTTPFinalListener::OnDataAvailable

[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHTTPResponseListener.cpp,
line 1232]

   InterceptStreamListener::OnDataAvailable

[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsCachedNetData.cpp, line 1165]

   nsHTTPServerListener::OnDataAvailable

[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHTTPResponseListener.cpp,
line 557]

   nsOnDataAvailableEvent::HandleEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line 407]

   nsStreamListenerEvent::HandlePLEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line 106]

   PL_HandleEvent

[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 588]

   PL_ProcessPendingEvents

[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 547]

   _md_EventReceiverProc

[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 1045]

   USER32.dll + 0x1820 (0x77e71820)

Comment 13

[Asa Dotzler [:asa]]

oops, wrong bug.  sorry. ignore that last comment.

Comment 14

[Asa Dotzler [:asa]]

after re-reading these few bugs I realized this bug is actaully not supposed to
depend on bug 43535. Removing dependency.  This is a spin off requesting that
the current file|quit mechanisms be evaluated and maybe re-implimented.

Comment 15

[Marko Macek]

I disagree. That would be a workaround solution... It's the scalability problem
that needs to be fixed (I have no problem with exit taking linear time 9 (about
O(#windows)), but exponential is a bit too much).

Comment 16

[Asa Dotzler [:asa]]

well, someone else needs to take this bug since I don't understand it well enough.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I just did a profile of this (note to self: file-quit-20-windows-rt1ms) - over
half the time is spent in JS GC (most in gc_mark_node).  It's probably a good
idea to do JS GC when closing windows, in general, because you should release
stuff when you close the window.  It ends up being O(N^2) on a file|quit (when
you close all the windows) since it should be O(N) on the number of windows open
at a given time.  (And there's even more stuff in JS if you leak stuff through
JS...)

Comment 18

[Marko Macek]

I don't think it's a good idea to always force GC anywhere. Except when out of
mem...

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think you have to force GC when you destroy a context to clear its newborns
(last created object of each type, which are rooted from GC) so they can be
GCed.  But maybe there ought to be a way around that...

It seems we're calling GC twice when we destroy a window:
 + once from GlobalWindowImpl::SetNewDocument, which calls nsJSContext::GC()
which calls JS_GC which calls js_ForceGC (which clears the newborns) which calls
js_GC
 + once from nsJSContext::~nsJSContext which calls JS_DestroyContext which calls
js_ForceGC (which clears the newborns) which calls js_GC
 
I suspect (hope) the first of these could be avoided, at least when aDocument is
null (and perhaps the context should be released right there to cause the second
GC sooner).

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Assigning this to DOM so it gets attention.  cc:ing brendan too.

Comment 21

[ekrock's old account (dead)]

FUTURE.

Comment 22

[Brendan Eich [:brendan]]

My fix to bug 49816 should have improved GC performance noticeably, already.

dbaron wrote:
>I think you have to force GC when you destroy a context to clear its newborns
>(last created object of each type, which are rooted from GC) so they can be
>GCed.  But maybe there ought to be a way around that...

Just call JS_DestroyContextNoGC and the newborns will disappear from the GC's 
sight along with the context that's destroyed.  You'll want to call JS_GC 
eventually, after destroying some large number of contexts, or once all windows 
are gone (quitting requires memory too, so you want to GC at least once, maybe 
more than once).

/be

Comment 23

[selmer (gone)]

With 3/8 04 build, on WinNT, 500 Mhz, 256 Mb machine I opened 40 windows and
then used ^Q to exit.  It took about a minute to close all the windows.  One
thing I noticed is that we spent most of that time changing the focus to the
previous window and drawing it as active.

Would it make sense to optimize this during shutdown?  There's no need to give
focus to a window that's about to close.  Avoiding focus changes would make this
case go a lot faster, but I don't know if normal usage patterns include large
numbers of windows being closed on exit.

As a point of reference, 4.x closes 50 windows with ^Q in about 1 second.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I took out the line that focuses the window before closing it (why did we
do that?), and it doesn't seem to speed things up that much, at least for
me on Linux.  (It could vary by platform -- what window.focus() does may
vary by platform.)

But it might not be a bad idea anyway.

And aren't there two bugs on this topic?  Because I thought the other one
has been determined to be an RDF problem and is now assigned to waterson.

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: not focus window before closing

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

This is related closely to bug 104127, which has up-to-date jprofs.

Comment 27

[selmer (gone)]

Did the "not focus" patch ever go in a build?  I'd like to test it on windows,
but I need someone to give me the bits.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The code that it patched doesn't exist anymore.

Comment 29

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 30

[Andrew Hagen]

*** Bug 39810 has been marked as a duplicate of this bug. ***

Comment 31

[Andrew Hagen]

*** Bug 64981 has been marked as a duplicate of this bug. ***

Comment 32

[Paul Wyskoczka]

adt: nsbeta1-

Comment 33

[Cees T.]

I think bug 44535 mentioned in comment 11 fixed the focus issue. Bug 354087 comment 4 mentions slow shutdown due to extreme swapping. Is this bug still alive?

Comment 34

[Wayne Mery (:wsmwk)]

(In reply to comment #33)
> Bug 354087 comment 4 mentions slow shutdown due to extreme swapping. Is this bug still alive?

shouldn't be - Bug 354087 was fixed (on trunk) by Bug 342810.
ditto bug 83421.

cc-ers, is this problem gone for you on trunk?

Comment 35

[Wayne Mery (:wsmwk)]

dbaron in comment #24
>...
> And aren't there two bugs on this topic?  Because I thought the other one
> has been determined to be an RDF problem and is now assigned to waterson.

virtually every major rdf bug from this era assigned to waterson is resolved, execpt bug 84187.

bug 127611 related ?

Comment 36

[Marko Macek]

This no longer occurs.

One of the reasons at the time was JS GC forced at each window close.