Last Comment Bug 735014 - GCs triggered from pagehide can interfere with the loading of another web page
: GCs triggered from pagehide can interfere with the loading of another web page
Status: RESOLVED FIXED
[Snappy]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla13
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on: 735099
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-12 13:32 PDT by :Ehsan Akhgari (out sick)
Modified: 2012-12-03 14:28 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.28 KB, patch)
2012-03-12 16:13 PDT, Bill McCloskey (:billm)
bugs: review+
Details | Diff | Review

Description :Ehsan Akhgari (out sick) 2012-03-12 13:32:01 PDT
Here's the scenario.  You click a link, this code <http://dxr.lanedo.com/mozilla-central/layout/base/nsDocumentViewer.cpp.html#l1292> schedules a GC in 4 seconds, then you start waiting on the network, etc, and in about the time when you have something to do in order to respond to the user's request, the GC timer fires and you get unresponsive if you're doing a big GC (which is possible since a previous page has been unloaded.)

I don't have a very good solution for this.  I've seen this in profiles where I have been trying to measure why loading a page is slow, and I have seen this taking almost all of the time that we spend away from the event loop.
Comment 1 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-12 13:37:59 PDT
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp?rev=5789df0e14d1#1285

Once we have iGC working, that would trigger just the start of incremental GC, and profiles
would look quite different. Big GCs shouldn't happen anymore, in general.
Comment 2 :Ehsan Akhgari (out sick) 2012-03-12 15:49:28 PDT
So do you recommend us closing this bug then?
Comment 3 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-12 15:58:16 PDT
Could we wait for iGC to work reliably, and re-test.

But, if that 4 second timer is bad, we could perhaps create a longer timer in this case.
We should do GC at some point, but it could be delayed. And it should run only if GC hasn't already
run after pagehide.
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-12 16:03:39 PDT
PokeGC takes 2nd parameter which is the time in ms.
Could you do something like NS_GC_DELAY*2 there?
NS_GC_DELAY should move to .h file.

Or check the reason in PokeGC, and if it is js::gcreason::PAGE_HIDE, do a longer delay.
Comment 5 Bill McCloskey (:billm) 2012-03-12 16:13:21 PDT
Created attachment 605206 [details] [diff] [review]
patch

Does the first thing, so now we're waiting 8 seconds instead of 4.
Comment 6 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-03-12 16:20:51 PDT
Comment on attachment 605206 [details] [diff] [review]
patch

Let's try this. And once iGC is there, we can remove this.

Could you make this bug to depend on the iGC bug, so that we don't forget.
Comment 7 Bill McCloskey (:billm) 2012-03-12 16:28:38 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/58ebf0fed1e8

I made a new bug for re-enabling incremental GC, and now this bug depends on that.
Comment 8 Marco Bonardo [::mak] 2012-03-13 05:42:53 PDT
https://hg.mozilla.org/mozilla-central/rev/58ebf0fed1e8

Note You need to log in before you can comment on or make changes to this bug.