Bug 821371, part 1 - Don't finish GC when CCTimerFired calls CycleCollectNow. r=smaug
authorAndrew McCreight <amccreight@mozilla.com>
Mon, 31 Dec 2012 15:54:37 -0500
changeset 126384 faf5e1fdde8873a60b04bb86bf9d0a1f8bdda7e5
parent 126383 5718a26eb442da4dc52b5666db66d65ff8fe4735
child 126385 8c4236dde5e31ad86a6449e7bcacfae5ab60a32b
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs821371
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 821371, part 1 - Don't finish GC when CCTimerFired calls CycleCollectNow. r=smaug
dom/base/nsJSEnvironment.cpp
js/xpconnect/src/XPCComponents.cpp
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -3060,19 +3060,30 @@ DoMergingCC(bool aForced)
   } else {
     sMergedInARow = 0;
     return false;
   }
 
 }
 
 static void
+FinishAnyIncrementalGC()
+{
+  if (sCCLockedOut) {
+    // We're in the middle of an incremental GC, so finish it.
+    js::PrepareForIncrementalGC(nsJSRuntime::sRuntime);
+    js::FinishIncrementalGC(nsJSRuntime::sRuntime, js::gcreason::CC_FORCED);
+  }
+}
+
+static void
 FireForgetSkippable(uint32_t aSuspected, bool aRemoveChildless)
 {
   PRTime startTime = PR_Now();
+  FinishAnyIncrementalGC();
   nsCycleCollector_forgetSkippable(aRemoveChildless);
   sPreviousSuspectedCount = nsCycleCollector_suspectedCount();
   ++sCleanupsSinceLastGC;
   PRTime delta = PR_Now() - startTime;
   if (sMinForgetSkippableTime > delta) {
     sMinForgetSkippableTime = delta;
   }
   if (sMaxForgetSkippableTime < delta) {
@@ -3088,21 +3099,17 @@ void
 nsJSContext::CycleCollectNow(nsICycleCollectorListener *aListener,
                              int32_t aExtraForgetSkippableCalls,
                              bool aForced)
 {
   if (!NS_IsMainThread()) {
     return;
   }
 
-  if (sCCLockedOut) {
-    // We're in the middle of an incremental GC; finish it first
-    js::PrepareForIncrementalGC(nsJSRuntime::sRuntime);
-    js::FinishIncrementalGC(nsJSRuntime::sRuntime, js::gcreason::CC_FORCED);
-  }
+  FinishAnyIncrementalGC();
 
   SAMPLE_LABEL("GC", "CycleCollectNow");
 
   KillCCTimer();
 
   PRTime start = PR_Now();
 
   uint32_t suspected = nsCycleCollector_suspectedCount();
@@ -3284,48 +3291,50 @@ CCTimerFired(nsITimer *aTimer, void *aCl
     PRTime now = PR_Now();
     if (sCCLockedOutTime == 0) {
       sCCLockedOutTime = now;
       return;
     }
     if (now - sCCLockedOutTime < NS_MAX_CC_LOCKEDOUT_TIME) {
       return;
     }
-
-    // Finish the current incremental GC
-    js::PrepareForIncrementalGC(nsJSRuntime::sRuntime);
-    js::FinishIncrementalGC(nsJSRuntime::sRuntime, js::gcreason::CC_FORCED);
   }
 
   ++sCCTimerFireCount;
 
   // 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 run the CC.
   const uint32_t numEarlyTimerFires = ccDelay / NS_CC_SKIPPABLE_DELAY - 2;
   bool isLateTimerFire = sCCTimerFireCount > numEarlyTimerFires;
   uint32_t suspected = nsCycleCollector_suspectedCount();
   if (isLateTimerFire && ShouldTriggerCC(suspected)) {
     if (sCCTimerFireCount == numEarlyTimerFires + 1) {
       FireForgetSkippable(suspected, true);
       if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) {
         // Our efforts to avoid a CC have failed, so we return to let the
         // timer fire once more to trigger a CC.
+        MOZ_ASSERT(!sCCLockedOut);
         return;
       }
     } else {
       // We are in the final timer fire and still meet the conditions for
-      // triggering a CC.
+      // triggering a CC. Let CycleCollectNow finish the current IGC, if any,
+      // because that will allow us to include the GC time in the CC pause.
       nsJSContext::CycleCollectNow(nullptr, 0, false);
     }
   } else if ((sPreviousSuspectedCount + 100) <= suspected) {
-    // Only do a forget skippable if there are more than a few new objects.
-    FireForgetSkippable(suspected, false);
+      // Only do a forget skippable if there are more than a few new objects.
+      FireForgetSkippable(suspected, false);
   }
 
+  // If we were in the middle of an incremental GC, we should have finished
+  // it by now.
+  MOZ_ASSERT(!sCCLockedOut);
+
   if (isLateTimerFire) {
     ccDelay = NS_CC_DELAY;
 
     // We have either just run the CC or decided we don't want to run the CC
     // next time, so kill the timer.
     sPreviousSuspectedCount = 0;
     nsJSContext::KillCCTimer();
   }
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -4041,17 +4041,17 @@ nsXPCComponents_Utils::ForceGC()
     js::GCForReason(rt, js::gcreason::COMPONENT_UTILS);
     return NS_OK;
 }
 
 /* void forceCC (); */
 NS_IMETHODIMP
 nsXPCComponents_Utils::ForceCC()
 {
-    nsJSContext::CycleCollectNow(nullptr, 0);
+    nsJSContext::CycleCollectNow();
     return NS_OK;
 }
 
 /* void forceShrinkingGC (); */
 NS_IMETHODIMP
 nsXPCComponents_Utils::ForceShrinkingGC()
 {
     JSRuntime* rt = nsXPConnect::GetRuntimeInstance()->GetJSRuntime();