Bug 1599445 - Refactor CCRunnerFired function by writing it as a state machine r=mccr8

This makes the function into a state machine implemented with a switch statement and gets rid of the recursive call. This results in a little duplication of logic but hopefully it is easier to understand overall. What do you think?

Differential Revision: https://phabricator.services.mozilla.com/D54750

--HG--
extra : moz-landing-system : lando

dom/base/nsJSEnvironment.cpp

@@ -148,6 +148,8 @@ static const uint32_t kCCPurpleLimit = 200;
 
 // if you add statics here, add them to the list in StartupJSEnvironment
 
+enum class CCRunnerState { Inactive, EarlyTimer, LateTimer, FinalTimer };
+
 static nsITimer* sGCTimer;
 static nsITimer* sShrinkingGCTimer;
 static StaticRefPtr<IdleTaskRunner> sCCRunner;
@@ -161,6 +163,8 @@ static TimeStamp sLastForgetSkippableCycleEndTime;
 
 static TimeStamp sCurrentGCStartTime;
 
+static CCRunnerState sCCRunnerState = CCRunnerState::Inactive;
+static TimeDuration sCCDelay = kCCDelay;
 static bool sCCLockedOut;
 static TimeStamp sCCLockedOutTime;
 
@@ -171,7 +175,7 @@ static bool sHasRunGC;
 static uint32_t sCCollectedWaitingForGC;
 static uint32_t sCCollectedZonesWaitingForGC;
 static uint32_t sLikelyShortLivingObjectsNeedingGC;
-static int32_t sCCRunnerFireCount = 0;
+static int32_t sCCRunnerEarlyFireCount = 0;
 static uint32_t sPreviousSuspectedCount = 0;
 static uint32_t sCleanupsSinceLastGC = UINT32_MAX;
 static bool sNeedsFullCC = false;
@@ -1812,50 +1816,101 @@ static bool ShouldTriggerCC(uint32_t aSuspected) {
           TimeUntilNow(sLastCCEndTime) > kCCForced);
 }
 
+static inline bool ShouldFireForgetSkippable(uint32_t aSuspected) {
+  // Only do a forget skippable if there are more than a few new objects
+  // or we're doing the initial forget skippables.
+  return ((sPreviousSuspectedCount + 100) <= aSuspected) ||
+         sCleanupsSinceLastGC < kMajorForgetSkippableCalls;
+}
+
+static inline bool IsLastEarlyCCTimer(int32_t aCurrentFireCount) {
+  int32_t numEarlyTimerFires =
+      std::max(int32_t(sCCDelay / kCCSkippableDelay) - 2, 1);
+
+  return aCurrentFireCount >= numEarlyTimerFires;
+}
+
+static void ActivateCCRunner() {
+  MOZ_ASSERT(sCCRunnerState == CCRunnerState::Inactive);
+  sCCRunnerState = CCRunnerState::EarlyTimer;
+  sCCDelay = kCCDelay;
+  sCCRunnerEarlyFireCount = 0;
+}
+
 static bool CCRunnerFired(TimeStamp aDeadline) {
   if (sDidShutdown) {
     return false;
   }
 
-  static TimeDuration ccDelay = kCCDelay;
   if (sCCLockedOut) {
-    ccDelay = kCCDelay / int64_t(3);
-
     TimeStamp now = TimeStamp::Now();
     if (!sCCLockedOutTime) {
-      // Reset sCCRunnerFireCount so that we run forgetSkippable
-      // often enough before CC. Because of reduced ccDelay
-      // forgetSkippable will be called just a few times.
-      // kMaxCCLockedoutTime limit guarantees that we end up calling
+      // Reset our state so that we run forgetSkippable often enough before
+      // CC. Because of reduced sCCDelay forgetSkippable will be called just a
+      // few times. kMaxCCLockedoutTime limit guarantees that we end up calling
       // forgetSkippable and CycleCollectNow eventually.
-      sCCRunnerFireCount = 0;
+      sCCRunnerState = CCRunnerState::EarlyTimer;
+      sCCRunnerEarlyFireCount = 0;
+      sCCDelay = kCCDelay / int64_t(3);
       sCCLockedOutTime = now;
       return false;
     }
+
     if (now - sCCLockedOutTime < kMaxCCLockedoutTime) {
       return false;
     }
   }
 
-  ++sCCRunnerFireCount;
-
   bool didDoWork = false;
+  bool finished = false;
 
-  // During early timer fires, we only run forgetSkippable. During the first
-  // late timer fire, we decide if we are going to have a second and final
-  // late timer fire, where we may begin to run the CC. Should run at least one
-  // early timer fire to allow cleanup before the CC.
-  int32_t numEarlyTimerFires =
-      std::max(int32_t(ccDelay / kCCSkippableDelay) - 2, 1);
-  bool isLateTimerFire = sCCRunnerFireCount > numEarlyTimerFires;
   uint32_t suspected = nsCycleCollector_suspectedCount();
-  if (isLateTimerFire && ShouldTriggerCC(suspected)) {
-    if (sCCRunnerFireCount == numEarlyTimerFires + 1) {
-      FireForgetSkippable(suspected, true, aDeadline);
+
+  switch (sCCRunnerState) {
+    case CCRunnerState::EarlyTimer:
+      ++sCCRunnerEarlyFireCount;
+      if (IsLastEarlyCCTimer(sCCRunnerEarlyFireCount)) {
+        sCCRunnerState = CCRunnerState::LateTimer;
+      }
+
+      if (ShouldFireForgetSkippable(suspected)) {
+        FireForgetSkippable(suspected, /* aRemoveChildless = */ false,
+                            aDeadline);
         didDoWork = true;
-      if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
+        break;
+      }
+
+      if (aDeadline.IsNull()) {
+        break;
+      }
+
+      // If we're called during idle time, try to find some work to do by
+      // advancing to the next state, effectively bypassing some possible forget
+      // skippable calls.
+      MOZ_ASSERT(!didDoWork);
+
+      sCCRunnerState = CCRunnerState::LateTimer;
+      MOZ_FALLTHROUGH;
+
+    case CCRunnerState::LateTimer:
+      if (!ShouldTriggerCC(suspected) && ShouldFireForgetSkippable(suspected)) {
+        FireForgetSkippable(suspected, /* aRemoveChildless = */ false,
+                            aDeadline);
+        didDoWork = true;
+        finished = true;
+        break;
+      }
+
+      FireForgetSkippable(suspected, /* aRemoveChildless = */ true, aDeadline);
+      didDoWork = true;
+      if (!ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
+        finished = true;
+        break;
+      }
+
       // Our efforts to avoid a CC have failed, so we return to let the
       // timer fire once more to trigger a CC.
+      sCCRunnerState = CCRunnerState::FinalTimer;
 
       if (!aDeadline.IsNull() && TimeStamp::Now() < aDeadline) {
         // Clear content unbinder before the first CC slice.
@@ -1866,38 +1921,33 @@ static bool CCRunnerFired(TimeStamp aDeadline) {
           nsCycleCollector_doDeferredDeletion();
         }
       }
-        return didDoWork;
+
+      break;
+
+    case CCRunnerState::FinalTimer:
+      if (!ShouldTriggerCC(suspected) && ShouldFireForgetSkippable(suspected)) {
+        FireForgetSkippable(suspected, /* aRemoveChildless = */ false,
+                            aDeadline);
+        didDoWork = true;
+        finished = true;
+        break;
       }
-    } else {
+
       // We are in the final timer fire and still meet the conditions for
       // triggering a CC. Let RunCycleCollectorSlice finish the current IGC, if
       // any because that will allow us to include the GC time in the CC pause.
       nsJSContext::RunCycleCollectorSlice(aDeadline);
       didDoWork = true;
-    }
-  } else if (((sPreviousSuspectedCount + 100) <= suspected) ||
-             (sCleanupsSinceLastGC < kMajorForgetSkippableCalls)) {
-    // Only do a forget skippable if there are more than a few new objects
-    // or we're doing the initial forget skippables.
-    FireForgetSkippable(suspected, false, aDeadline);
-    didDoWork = true;
-  } else if (!isLateTimerFire && !aDeadline.IsNull()) {
-    MOZ_ASSERT(!didDoWork);
-    // If we're called during idle time, try to find some work to do by calling
-    // the method recursively, effectively bypassing some possible forget
-    // skippable calls.
-    sCCRunnerFireCount = numEarlyTimerFires;
-    return CCRunnerFired(aDeadline);
+      finished = true;
+      break;
+
+    default:
+      MOZ_CRASH("Unexpected CCRunner state");
   }
 
-  if (isLateTimerFire) {
-    ccDelay = kCCDelay;
-
-    // We have either just run the CC or decided we don't want to run the CC
-    // next time, so kill the timer.
+  if (finished) {
     sPreviousSuspectedCount = 0;
     nsJSContext::KillCCRunner();
-
     if (!didDoWork) {
       sLastForgetSkippableCycleEndTime = TimeStamp::Now();
     }
@@ -2092,11 +2142,11 @@ void nsJSContext::MaybePokeCC() {
   }
 
   if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
-    sCCRunnerFireCount = 0;
 
     // We can kill some objects before running forgetSkippable.
     nsCycleCollector_dispatchDeferredDeletion();
 
+    ActivateCCRunner();
     sCCRunner = IdleTaskRunner::Create(
         CCRunnerFired, "MaybePokeCC::CCRunnerFired",
         kCCSkippableDelay.ToMilliseconds(),
@@ -2138,6 +2188,7 @@ void nsJSContext::KillShrinkingGCTimer() {
 // static
 void nsJSContext::KillCCRunner() {
   sCCLockedOutTime = TimeStamp();
+  sCCRunnerState = CCRunnerState::Inactive;
   if (sCCRunner) {
     sCCRunner->Cancel();
     sCCRunner = nullptr;