Closed Bug 1296639 Opened 4 years ago Closed 4 years ago

There should be only one periodic full GC

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(2 files)

At the moment GCRuntime::maybePeriodicFullGC will trigger a full GC every 20 seconds if MaybeGC is called (which is called after running scripts from the browser).  However the browser also triggers its own full GC every 60 seconds.

We should remove one of these.
The one in DOMGCSliceCallback is triggered when doing a compartmental GC. What does maybePeriodicFullGC do? Or when is it called?
Remove SM's periodic full GC.  The embedding can still trigger such GCs if it decides it needs to (and gecko still does of course).
Assignee: nobody → jcoppeard
Attachment #8782969 - Flags: review?(sphink)
Attachment #8782969 - Flags: review?(sphink) → review+
(In reply to Olli Pettay [:smaug] from comment #1)
> The one in DOMGCSliceCallback is triggered when doing a compartmental GC.
> What does maybePeriodicFullGC do? Or when is it called?

maybePeriodicFullGC triggers a full (all-zones) GC every 20 seconds if there has been significant GC allocation or there are enough arenas to decommit.  

We already have other allocation based triggers though and we now decommit arenas at the end of GC anyway so I don't think this is required any more.  Telemetry shows this happens pretty rarely in practice too (0.17%).
Keywords: leave-open
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c82bf507f68b
Remove SpiderMonkey's periodic full GC r=sfink
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/c82bf507f68b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32c3c2cb3b92
Remove decommit threshold prefs r=terrence
Attached patch bug1296639-betaSplinter Review
Approval Request Comment
[Feature/regressing bug #]: Not a regression, this removes an unnecessary GC trigger.
[User impact if declined]: Requested by RyanVM to reduce intermittent test failures in bug 1293057.
[Describe test coverage new/current, TreeHerder]: On m-c since August 23rd.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Attachment #8795287 - Flags: review+
Attachment #8795287 - Flags: approval-mozilla-beta?
Comment on attachment 8795287 [details] [diff] [review]
bug1296639-beta

Fixes an intermittent (verified by RyanVM), Beta50+
Attachment #8795287 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.