In profiler data, we could see the heavy GC task before the first paint event. We are trying to improve our webpage startup time. Maybe we could have some heuristic rules for GC module that do not do the GC before the first paint. Profiler: https://cleopatra.io/#report=37b1b314384bfff047553f1995a62b608457f830&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A60317,%22end%22%3A60881%7D%5D&selection=0,3902
Steps to Reproduce: 1. Launch browser 2. Open a google doc with 200+ pages https://docs.google.com/document/d/1EPSmGqm2r4Qq42B4t1VOYacjTlL0JVuC8JSlUvoIhss/edit?usp=sharing
I think the way this works now is that when we set a new document or something we call nsJSContext::PokeGC(), which triggers a GC 4 seconds later. The idea is that this should happen after a page loads, but it looks like this page load takes around 9 seconds, so we end up GCing while the page is still loading.
We looked at this in Taipei last week, the GCReason is SET_NEW_DOCUMENT. Why do we GC there? That seems like it's always going to be in the page load path ...
IIRC we do that because of some really old historical reasons. So better to look for CVS blame.
Is that still possible? Bonsai is gone, IIRC.
FWIW, I can see the relevant GC call in http://184.108.40.206/viewvc/main/mozilla/dom/src/base/nsGlobalWindow.cpp?revision=1.1&view=markup#l277 Thu Jul 16 01:16:25 1998 UTC
(In reply to Kyle Huey [:khuey] (email@example.com) from comment #4) > We looked at this in Taipei last week, the GCReason is SET_NEW_DOCUMENT. > Why do we GC there? It doesn't GC immediately, it GCs in 4 seconds, so usually it will fire after the document is loaded, I'd think. We also poke the GC in nsDocumentViewer::LoadComplete() (which is a LOAD_END GC), so maybe that's enough and we can just drop SET_NEW_DOCUMENT?
Created attachment 8750050 [details] [diff] [review] skip the GC in nsGlobalWindow::SetNewDocument(). As comment 8, this patch try to skip the gc during the page loading. It's worth to test with this change. The GC module observes the memory-pressure event, so I think we sill don't hit the OOM problem.
Created attachment 8750051 [details] [diff] [review] show GC reason in gecko profiler from Kyle's hint, show the gc reason to profiler.
Should we try to land these patches Jon?
I did a try run and it looks ok: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be0157e180a966e09101873389d205b0c7b44123&selectedJob=81631861
Created attachment 8843572 [details] [diff] [review] bug1270056-remove-poke-gc smaug, do you think its OK to remove this?
Comment on attachment 8843572 [details] [diff] [review] bug1270056-remove-poke-gc ah, this might be fine, given that there is http://searchfox.org/mozilla-central/rev/8dcf6f318c3b0d8298d88b43bd6815405227dc93/layout/base/nsDocumentViewer.cpp#1088 So, I'm ok to try this. Please push to try with all the platforms and debug and opt builds.
(In reply to Olli Pettay [:smaug] (high review load) from comment #14) Try build here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1c16273355c538d686801f0496ae78be2340271&selectedJob=81886118 This mostly green bar a few failures that don't seem related to GC.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/f175c0117072 skip the GC in nsGlobalWindow::SetNewDocument() r=smaug