Bug 937751, part 4 - Incrementalize nsCycleCollector::Collect. r=smaug
authorAndrew McCreight <continuation@gmail.com>
Tue, 03 Dec 2013 10:47:47 -0800
changeset 174289 c6338d6ddd1fb6fef6be6de1a7c63e4864e05ae9
parent 174288 aa6afd008433e97833bf80811678b4c5178e945e
child 174290 d2a69e306fb5f8a0e7d24fa7e5cdee34a5add254
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs937751
milestone28.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 937751, part 4 - Incrementalize nsCycleCollector::Collect. r=smaug This patch makes it so that Collect takes a time budget that describes how much longer the collection can be run for. Then we run the current phase. Once this is done, we check whether we have exceeded our time budget or if we have finished a collection. If neither of those have happened, we run the cycle collector some more. If we're a manually triggered CC, and we were in the middle of an ICC when the CC started, then once the current CC is complete, we start a new CC immediately. This is needed to ensure that a manually specified listener is used, and to ensure that any garbage objects the caller expects to be collected are in fact collected. Note that in this patch we are always passing in an unlimited budget to Collect, so cycle collections will always be run to completion.
xpcom/base/nsCycleCollector.cpp
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -116,16 +116,17 @@
 #include "nsIConsoleService.h"
 #include "mozilla/Attributes.h"
 #include "nsICycleCollectorListener.h"
 #include "nsIMemoryReporter.h"
 #include "nsIFile.h"
 #include "nsMemoryInfoDumper.h"
 #include "xpcpublic.h"
 #include "GeckoProfiler.h"
+#include "js/SliceBudget.h"
 #include <stdint.h>
 #include <stdio.h>
 
 #include "mozilla/Likely.h"
 #include "mozilla/PoisonIOInterposer.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/ThreadLocal.h"
 
@@ -997,16 +998,18 @@ enum ccType {
 #ifdef MOZ_NUWA_PROCESS
 #include "ipc/Nuwa.h"
 #endif
 
 ////////////////////////////////////////////////////////////////////////
 // Top level structure for the cycle collector.
 ////////////////////////////////////////////////////////////////////////
 
+typedef js::SliceBudget SliceBudget;
+
 class nsCycleCollector : public MemoryMultiReporter
 {
     NS_DECL_ISUPPORTS
 
     bool mActivelyCollecting;
     // mScanInProgress should be false when we're collecting white objects.
     bool mScanInProgress;
     CycleCollectorResults mResults;
@@ -1054,16 +1057,17 @@ public:
 
     void Suspect(void *n, nsCycleCollectionParticipant *cp,
                  nsCycleCollectingAutoRefCnt *aRefCnt);
     uint32_t SuspectedCount();
     void ForgetSkippable(bool aRemoveChildlessNodes, bool aAsyncSnowWhiteFreeing);
     bool FreeSnowWhite(bool aUntilNoSWInPurpleBuffer);
 
     bool Collect(ccType aCCType,
+                 SliceBudget &aBudget,
                  nsICycleCollectorListener *aManualListener);
     void Shutdown();
 
     NS_IMETHOD CollectReports(nsIHandleReportCallback* aHandleReport,
                               nsISupports* aData);
 
     void SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
                              size_t *aObjectSize,
@@ -2704,46 +2708,84 @@ nsCycleCollector::CleanupAfterCollection
         mJSRuntime->EndCycleCollectionCallback(mResults);
     }
     mIncrementalPhase = IdlePhase;
 }
 
 void
 nsCycleCollector::ShutdownCollect()
 {
+    SliceBudget unlimitedBudget;
     for (uint32_t i = 0; i < DEFAULT_SHUTDOWN_COLLECTIONS; ++i) {
         NS_ASSERTION(i < NORMAL_SHUTDOWN_COLLECTIONS, "Extra shutdown CC");
-        if (!Collect(ShutdownCC, nullptr)) {
+        if (!Collect(ShutdownCC, unlimitedBudget, nullptr)) {
             break;
         }
     }
 }
 
 bool
 nsCycleCollector::Collect(ccType aCCType,
+                          SliceBudget &aBudget,
                           nsICycleCollectorListener *aManualListener)
 {
     CheckThreadSafety();
 
     // This can legitimately happen in a few cases. See bug 383651.
     if (mActivelyCollecting) {
         return false;
     }
     mActivelyCollecting = true;
 
-    MOZ_ASSERT(mIncrementalPhase == IdlePhase);
-
-    BeginCollection(aCCType, aManualListener);
-    MarkRoots();
-    ScanRoots();
-    bool collectedAny = CollectWhite();
-    CleanupAfterCollection();
+    bool startedIdle = (mIncrementalPhase == IdlePhase);
+    bool collectedAny = false;
+
+    // If the CC started idle, it will call BeginCollection, which
+    // will do FreeSnowWhite, so it doesn't need to be done here.
+    if (!startedIdle) {
+        FreeSnowWhite(true);
+    }
+
+    bool finished = false;
+    do {
+        switch (mIncrementalPhase) {
+        case IdlePhase:
+            BeginCollection(aCCType, aManualListener);
+            break;
+        case GraphBuildingPhase:
+            MarkRoots();
+            break;
+        case ScanAndCollectWhitePhase:
+            // We do ScanRoots and CollectWhite in a single slice to ensure
+            // that we won't unlink a live object if a weak reference is
+            // promoted to a strong reference after ScanRoots has finished.
+            // See bug 926533.
+            ScanRoots();
+            collectedAny = CollectWhite();
+            break;
+        case CleanupPhase:
+            CleanupAfterCollection();
+            finished = true;
+            break;
+        }
+    } while (!aBudget.checkOverBudget() && !finished);
+
     mActivelyCollecting = false;
 
-    MOZ_ASSERT(mIncrementalPhase == IdlePhase);
+    if (aCCType != ScheduledCC && !startedIdle) {
+        // We were in the middle of an incremental CC (using its own listener).
+        // Somebody has forced a CC, so after having finished out the current CC,
+        // run the CC again using the new listener.
+        MOZ_ASSERT(mIncrementalPhase == IdlePhase);
+        if (Collect(aCCType, aBudget, aManualListener)) {
+            collectedAny = true;
+        }
+    }
+
+    MOZ_ASSERT_IF(aCCType != ScheduledCC, mIncrementalPhase == IdlePhase);
 
     return collectedAny;
 }
 
 // Don't merge too many times in a row, and do at least a minimum
 // number of unmerged CCs in a row.
 static const uint32_t kMinConsecutiveUnmerged = 3;
 static const uint32_t kMaxConsecutiveMerged = 3;
@@ -3197,30 +3239,32 @@ nsCycleCollector_collect(nsICycleCollect
 {
     CollectorData *data = sCollectorData.get();
 
     // We should have started the cycle collector by now.
     MOZ_ASSERT(data);
     MOZ_ASSERT(data->mCollector);
 
     PROFILER_LABEL("CC", "nsCycleCollector_collect");
-    data->mCollector->Collect(ManualCC, aManualListener);
+    SliceBudget unlimitedBudget;
+    data->mCollector->Collect(ManualCC, unlimitedBudget, aManualListener);
 }
 
 void
 nsCycleCollector_scheduledCollect()
 {
     CollectorData *data = sCollectorData.get();
 
     // We should have started the cycle collector by now.
     MOZ_ASSERT(data);
     MOZ_ASSERT(data->mCollector);
 
     PROFILER_LABEL("CC", "nsCycleCollector_scheduledCollect");
-    data->mCollector->Collect(ScheduledCC, nullptr);
+    SliceBudget unlimitedBudget;
+    data->mCollector->Collect(ScheduledCC, unlimitedBudget, nullptr);
 }
 
 void
 nsCycleCollector_shutdown()
 {
     CollectorData *data = sCollectorData.get();
 
     if (data) {