Backed out 4 changesets (bug 1725539) for performance & crash regressions. CLOSED TREE

Backed out changeset a11b32e4aa7b (bug 1725539)
Backed out changeset fa262244a063 (bug 1725539)
Backed out changeset b31d0c7c8f5e (bug 1725539)
Backed out changeset c6d75ecdad79 (bug 1725539)

Author: Cristian Tuns

dom/base/CCGCScheduler.cpp

@@ -9,103 +9,6 @@
 #include "mozilla/dom/ScriptSettings.h"
 #include "nsRefreshDriver.h"
 
-/*
- * GC Scheduling from Firefox
- * ==========================
- *
- * See also GC Scheduling from SpiderMonkey's perspective here:
- * https://searchfox.org/mozilla-central/source/js/src/gc/Scheduling.h
- *
- * From Firefox's perspective GCs can start in 5 different ways:
- *
- *  * The JS engine just starts doing a GC for its own reasons (see above).
- *    Firefox finds out about these via a callback in nsJSEnvironment.cpp
- *  * PokeGC()
- *  * PokeFullGC()
- *  * PokeShrinkingGC()
- *  * memory-pressure GCs (via a listener in nsJSEnvironment.cpp).
- *
- * PokeGC
- * ------
- *
- * void CCGCScheduler::PokeGC(JS::GCReason aReason, JSObject* aObj,
- *                            TimeDuration aDelay)
- *
- * PokeGC provides a way for callers to say "Hey, there may be some memory
- * associated with this object (via Zone) you can collect."  PokeGC will:
- *  * add the zone to a set,
- *  * set flags including what kind of GC to run (SetWantMajorGC),
- *  * then creates the mGCRunner with a short delay.
- *
- * The delay can allow other calls to PokeGC to add their zones so they can
- * be collected together.
- *
- * See below for what happens when mGCRunner fires.
- *
- * PokeFullGC
- * ----------
- *
- * void CCGCScheduler::PokeFullGC()
- *
- * PokeFullGC will create a timer that will initiate a "full" (all zones)
- * collection.  This is usually used after a regular collection if a full GC
- * seems like a good idea (to collect inter-zone references).
- *
- * When the timer fires it will:
- *  * set flags (SetWantMajorGC),
- *  * start the mGCRunner with zero delay.
- *
- * See below for when mGCRunner fires.
- *
- * PokeShrinkingGC
- * ---------------
- *
- * void CCGCScheduler::PokeShrinkingGC()
- *
- * PokeShrinkingGC is called when Firefox's user is inactive.
- * Like PokeFullGC, PokeShrinkingGC uses a timer, but the timeout is longer
- * which should prevent the ShrinkingGC from starting if the user only
- * glances away for a brief time.  When the timer fires it will:
- *
- *  * set flags (SetWantMajorGC),
- *  * create the mGCRunner.
- *
- * There is a check if the user is still inactive in GCRunnerFired), if the
- * user has become active the shrinking GC is canceled and either a regular
- * GC (if requested, see mWantAtLeastRegularGC) or no GC is run.
- *
- * When mGCRunner fires
- * --------------------
- *
- * When mGCRunner fires it calls GCRunnerFired.  This starts in the
- * WaitToMajorGC state:
- *
- *  * If this is a parent process it jumps to the next state
- *  * If this is a content process it will ask the parent if now is a good
- *      time to do a GC.  (MayGCNow)
- *  * kill the mGCRunner
- *  * Exit
- *
- * Meanwhile the parent process will queue GC requests so that not too many
- * are running in parallel overwhelming the CPU cores (see
- * IdleSchedulerParent).
- *
- * When the promise from MayGCNow is resolved it will set some
- * state (NoteReadyForMajorGC) and restore the mGCRunner.
- *
- * When the mGCRunner runs a second time (or this is the parent process and
- * which jumped over the above logic.  It will be in the StartMajorGC state.
- * It will initiate the GC for real, usually.  If it's a shrinking GC and the
- * user is now active again it may abort.  See GCRunnerFiredDoGC().
- *
- * The runner will then run the first slice of the garbage collection.
- * Later slices are also run by the runner, the final slice kills the runner
- * from the GC callback in nsJSEnvironment.cpp.
- *
- * There is additional logic in the code to handle concurrent requests of
- * various kinds.
- */
-
 namespace mozilla {
 
 void CCGCScheduler::NoteGCBegin() {
@@ -156,7 +59,6 @@ void CCGCScheduler::NoteWontGC() {
 
 bool CCGCScheduler::GCRunnerFired(TimeStamp aDeadline) {
   MOZ_ASSERT(!mDidShutdown, "GCRunner still alive during shutdown");
-  MOZ_ASSERT(!mHaveAskedParent, "GCRunner alive after asking the parent");
 
   GCRunnerStep step = GetNextGCRunnerAction();
   switch (step.mAction) {
@@ -171,12 +73,10 @@ bool CCGCScheduler::GCRunnerFired(TimeStamp aDeadline) {
         break;
       }
 
-      mHaveAskedParent = true;
       KillGCRunner();
       mbPromise->Then(
           GetMainThreadSerialEventTarget(), __func__,
           [this](bool aMayGC) {
-            mHaveAskedParent = false;
             if (aMayGC) {
               if (!NoteReadyForMajorGC()) {
                 // Another GC started and maybe completed while waiting.
@@ -194,7 +94,6 @@ bool CCGCScheduler::GCRunnerFired(TimeStamp aDeadline) {
             }
           },
           [this](mozilla::ipc::ResponseRejectReason r) {
-            mHaveAskedParent = false;
             if (!InIncrementalGC()) {
               KillGCRunner();
               NoteWontGC();
@@ -389,8 +288,8 @@ void CCGCScheduler::PokeGC(JS::GCReason aReason, JSObject* aObj,
     SetNeedsFullGC();
   }
 
-  if (mGCRunner || mHaveAskedParent) {
-    // There's already a GC runner, there or will be, so just return.
+  if (mGCRunner) {
+    // There's already a runner for GC'ing, just return
     return;
   }
 
@@ -416,7 +315,7 @@ void CCGCScheduler::PokeGC(JS::GCReason aReason, JSObject* aObj,
 }
 
 void CCGCScheduler::EnsureGCRunner(TimeDuration aDelay) {
-  if (mGCRunner || mHaveAskedParent) {
+  if (mGCRunner) {
     return;
   }
 

dom/base/CCGCScheduler.h

@@ -166,39 +166,18 @@ class CCGCScheduler {
   void SetWantMajorGC(JS::GCReason aReason) {
     MOZ_ASSERT(aReason != JS::GCReason::NO_REASON);
 
-    // If the GC being requested is not a shrinking GC set this flag.
-    // If/when the shrinking GC timer fires but the user is active we check
-    // this flag before canceling the GC, so as not to cancel the
-    // non-shrinking GC being requested here.
-    if (aReason != JS::GCReason::USER_INACTIVE) {
+    if (mMajorGCReason != JS::GCReason::NO_REASON &&
+        mMajorGCReason != JS::GCReason::USER_INACTIVE &&
+        aReason != JS::GCReason::USER_INACTIVE) {
       mWantAtLeastRegularGC = true;
     }
+    mMajorGCReason = aReason;
 
     // Force full GCs when called from reftests so that we collect dead zones
     // that have not been scheduled for collection.
     if (aReason == JS::GCReason::DOM_WINDOW_UTILS) {
       SetNeedsFullGC();
     }
-
-    // USER_INACTIVE trumps everything,
-    // FULL_GC_TIMER trumps everything except USER_INACTIVE,
-    // all other reasons just use the latest reason.
-    switch (aReason) {
-      case JS::GCReason::USER_INACTIVE:
-        mMajorGCReason = aReason;
-        break;
-      case JS::GCReason::FULL_GC_TIMER:
-        if (mMajorGCReason != JS::GCReason::USER_INACTIVE) {
-          mMajorGCReason = aReason;
-        }
-        break;
-      default:
-        if (mMajorGCReason != JS::GCReason::USER_INACTIVE &&
-            mMajorGCReason != JS::GCReason::FULL_GC_TIMER) {
-          mMajorGCReason = aReason;
-        }
-        break;
-    }
   }
 
   // Ensure that the current runner does a cycle collection, and trigger a GC
@@ -409,10 +388,6 @@ class CCGCScheduler {
   // duration (or until it goes too long and is finished synchronously.)
   bool mInIncrementalGC = false;
 
-  // We've asked the parent process if now is a good time to GC (do not ask
-  // again).
-  bool mHaveAskedParent = false;
-
   // The parent process is ready for us to do a major GC.
   bool mReadyForMajorGC = false;
 

js/src/gc/Scheduling.h

@@ -10,9 +10,6 @@
  * GC Scheduling Overview
  * ======================
  *
- * See also GC scheduling from Firefox's perspective here:
- * https://searchfox.org/mozilla-central/source/dom/base/CCGCScheduler.cpp
- *
  * Scheduling GC's in SpiderMonkey/Firefox is tremendously complicated because
  * of the large number of subtle, cross-cutting, and widely dispersed factors
  * that must be taken into account. A summary of some of the more important