Backed out changeset 300f5b7d72e1 (bug 897433) for intermittent Linux dromaeo crashes.
authorRyan VanderMeulen <ryanvm@gmail.com>
Thu, 25 Jul 2013 21:18:20 -0400
changeset 152373 7d9fad4b940ffb5275ef427ba3ecb1e5c37deed8
parent 152372 924c4d59db8b27892cfcdff998c97ec0111a4689
child 152374 aacf8f0ef0175e92f73935509de362e1cfc0916a
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs897433
milestone25.0a1
backs out300f5b7d72e1d9306b251aba446557f34eec8057
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
Backed out changeset 300f5b7d72e1 (bug 897433) for intermittent Linux dromaeo crashes.
dom/base/nsJSEnvironment.cpp
toolkit/components/telemetry/Histograms.json
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsCycleCollector.h
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -130,18 +130,16 @@ static PRLogModuleInfo* gJSDiagnostics;
 // Trigger a CC if the purple buffer exceeds this size when we check it.
 #define NS_CC_PURPLE_LIMIT          200
 
 #define JAVASCRIPT nsIProgrammingLanguage::JAVASCRIPT
 
 // Large value used to specify that a script should run essentially forever
 #define NS_UNLIMITED_SCRIPT_RUNTIME (0x40000000LL << 32)
 
-#define NS_MAJOR_FORGET_SKIPPABLE_CALLS 2
-
 // if you add statics here, add them to the list in nsJSRuntime::Startup
 
 static nsITimer *sGCTimer;
 static nsITimer *sShrinkGCBuffersTimer;
 static nsITimer *sCCTimer;
 static nsITimer *sFullGCTimer;
 static nsITimer *sInterSliceGCTimer;
 
@@ -2499,19 +2497,17 @@ FinishAnyIncrementalGC()
   }
 }
 
 static void
 FireForgetSkippable(uint32_t aSuspected, bool aRemoveChildless)
 {
   PRTime startTime = PR_Now();
   FinishAnyIncrementalGC();
-  bool earlyForgetSkippable =
-    sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS;
-  nsCycleCollector_forgetSkippable(aRemoveChildless, earlyForgetSkippable);
+  nsCycleCollector_forgetSkippable(aRemoveChildless);
   sPreviousSuspectedCount = nsCycleCollector_suspectedCount();
   ++sCleanupsSinceLastGC;
   PRTime delta = PR_Now() - startTime;
   if (sMinForgetSkippableTime > delta) {
     sMinForgetSkippableTime = delta;
   }
   if (sMaxForgetSkippableTime < delta) {
     sMaxForgetSkippableTime = delta;
@@ -2551,19 +2547,18 @@ nsJSContext::CycleCollectNow(nsICycleCol
 
   KillCCTimer();
 
   uint32_t suspected = nsCycleCollector_suspectedCount();
   bool ranSyncForgetSkippable = false;
 
   // Run forgetSkippable synchronously to reduce the size of the CC graph. This
   // is particularly useful if we recently finished a GC.
-  if (sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS &&
-      aExtraForgetSkippableCalls >= 0) {
-    while (sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS) {
+  if (sCleanupsSinceLastGC < 2 && aExtraForgetSkippableCalls >= 0) {
+    while (sCleanupsSinceLastGC < 2) {
       FireForgetSkippable(nsCycleCollector_suspectedCount(), false);
       ranSyncForgetSkippable = true;
     }
   }
 
   for (int32_t i = 0; i < aExtraForgetSkippableCalls; ++i) {
     FireForgetSkippable(nsCycleCollector_suspectedCount(), false);
     ranSyncForgetSkippable = true;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -81,22 +81,16 @@
     "high": "10000",
     "n_buckets": 50,
     "description": "Time spent on one ContentUnbinder (ms)"
   },
   "CYCLE_COLLECTOR_OOM": {
     "kind": "flag",
     "description": "Set if the cycle collector ran out of memory at some point"
   },
-  "CYCLE_COLLECTOR_ASYNC_SNOW_WHITE_FREEING": {
-    "kind": "exponential",
-    "high": "10000",
-    "n_buckets": 50,
-    "description": "Time spent on one asynchronous SnowWhite freeing (ms)"
-  },
   "FORGET_SKIPPABLE_MAX": {
     "kind": "exponential",
     "high": "10000",
     "n_buckets": 50,
     "description": "Max time spent on one forget skippable (ms)"
   },
   "GC_REASON_2": {
     "kind": "enumerated",
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -770,25 +770,24 @@ public:
     void UnmarkRemainingPurple(Block *b)
     {
         UnmarkRemainingPurpleVisitor visitor;
         b->VisitEntries(*this, visitor);
     }
 
     void SelectPointers(GCGraphBuilder &builder);
 
-    // RemoveSkippable removes entries from the purple buffer synchronously
-    // (1) if aAsyncSnowWhiteFreeing is false and nsPurpleBufferEntry::mRefCnt is 0 or
-    // (2) if the object's nsXPCOMCycleCollectionParticipant::CanSkip() returns true or
-    // (3) if nsPurpleBufferEntry::mRefCnt->IsPurple() is false.
-    // (4) If removeChildlessNodes is true, then any nodes in the purple buffer
-    //     that will have no children in the cycle collector graph will also be
-    //     removed. CanSkip() may be run on these children.
+    // RemoveSkippable removes entries from the purple buffer if
+    // nsPurpleBufferEntry::mRefCnt is 0 or if the object's
+    // nsXPCOMCycleCollectionParticipant::CanSkip() returns true or
+    // if nsPurpleBufferEntry::mRefCnt->IsPurple() is false.
+    // If removeChildlessNodes is true, then any nodes in the purple buffer
+    // that will have no children in the cycle collector graph will also be
+    // removed. CanSkip() may be run on these children.
     void RemoveSkippable(bool removeChildlessNodes,
-                         bool aAsyncSnowWhiteFreeing,
                          CC_ForgetSkippableCallback aCb);
 
     nsPurpleBufferEntry* NewEntry()
     {
         if (!mFreeList) {
             Block *b = new Block;
             StartBlock(b);
 
@@ -1033,17 +1032,17 @@ public:
         mForgetSkippableCB = aForgetSkippableCB;
     }
 
     void SelectPurple(GCGraphBuilder &builder);
     void MarkRoots(GCGraphBuilder &builder);
     void ScanRoots();
     void ScanWeakMaps();
 
-    void ForgetSkippable(bool aRemoveChildlessNodes, bool aAsyncSnowWhiteFreeing);
+    void ForgetSkippable(bool removeChildlessNodes);
 
     // returns whether anything was collected
     bool CollectWhite(nsICycleCollectorListener *aListener);
 
     nsCycleCollector(CCThreadingModel aModel);
     ~nsCycleCollector();
 
     nsresult Init();
@@ -1068,22 +1067,21 @@ public:
     void FixGrayBits(bool aForceGC);
     bool ShouldMergeZones(ccType aCCType);
     void CleanupAfterCollection();
 
     // Start and finish an individual collection.
     bool BeginCollection(ccType aCCType, nsICycleCollectorListener *aListener);
     bool FinishCollection(nsICycleCollectorListener *aListener);
 
-    bool FreeSnowWhite(bool aUntilNoSWInPurpleBuffer);
+    void FreeSnowWhite(bool aUntilNoSWInPurpleBuffer);
 
     // If there is a cycle collector available in the current thread,
-    // this calls FreeSnowWhite(false). Returns true if some
-    // snow-white objects were found.
-    static bool TryToFreeSnowWhite();
+    // this calls FreeSnowWhite(false).
+    static void TryToFreeSnowWhite();
 
     uint32_t SuspectedCount();
     void Shutdown();
 
     void ClearGraph()
     {
         mGraph.mNodes.Clear();
         mGraph.mEdges.Clear();
@@ -2145,43 +2143,16 @@ ChildFinder::NoteJSChild(void *child)
 static bool
 MayHaveChild(void *o, nsCycleCollectionParticipant* cp)
 {
     ChildFinder cf;
     cp->Traverse(o, cf);
     return cf.MayHaveChild();
 }
 
-class AsyncFreeSnowWhite : public nsRunnable
-{
-public:
-  NS_IMETHOD Run()
-  {
-      PRTime start = PR_Now();
-      bool hadSnowWhiteObjects = nsCycleCollector::TryToFreeSnowWhite();
-      Telemetry::Accumulate(Telemetry::CYCLE_COLLECTOR_ASYNC_SNOW_WHITE_FREEING,
-                            uint32_t(PR_Now() - start) / PR_USEC_PER_MSEC);
-      if (hadSnowWhiteObjects && !mContinuation) {
-          mContinuation = true;
-          NS_DispatchToCurrentThread(this);
-      }
-      return NS_OK;
-  }
-
-  static void Dispatch(bool aContinuation = false)
-  {
-      nsRefPtr<AsyncFreeSnowWhite> ev = new AsyncFreeSnowWhite(aContinuation);
-      NS_DispatchToCurrentThread(ev);
-  }
-private:
-  AsyncFreeSnowWhite(bool aContinuation) : mContinuation(aContinuation) {}
-
-  bool mContinuation;
-};
-
 struct SnowWhiteObject
 {
   void* mPointer;
   nsCycleCollectionParticipant* mParticipant;
   nsCycleCollectingAutoRefCnt* mRefCnt;
 };
 
 class SnowWhiteKiller
@@ -2233,118 +2204,117 @@ public:
 private:
     FallibleTArray<SnowWhiteObject> mObjects;
 };
 
 class RemoveSkippableVisitor : public SnowWhiteKiller
 {
 public:
     RemoveSkippableVisitor(uint32_t aMaxCount, bool aRemoveChildlessNodes,
-                           bool aAsyncSnowWhiteFreeing,
                            CC_ForgetSkippableCallback aCb)
-        : SnowWhiteKiller(aAsyncSnowWhiteFreeing ? 0 : aMaxCount),
+        : SnowWhiteKiller(aMaxCount),
           mRemoveChildlessNodes(aRemoveChildlessNodes),
-          mAsyncSnowWhiteFreeing(aAsyncSnowWhiteFreeing),
-          mDispatchedDeferredDeletion(false),
           mCallback(aCb)
     {}
 
     ~RemoveSkippableVisitor()
     {
         // Note, we must call the callback before SnowWhiteKiller calls
         // DeleteCycleCollectable!
         if (mCallback) {
             mCallback();
         }
-        if (HasSnowWhiteObjects()) {
-            // Effectively a continuation.
-            AsyncFreeSnowWhite::Dispatch(true);
-        }
     }
 
     void
     Visit(nsPurpleBuffer &aBuffer, nsPurpleBufferEntry *aEntry)
     {
         MOZ_ASSERT(aEntry->mObject, "null mObject in purple buffer");
         if (!aEntry->mRefCnt->get()) {
-            if (!mAsyncSnowWhiteFreeing) {
-                SnowWhiteKiller::Visit(aBuffer, aEntry);
-            } else if (!mDispatchedDeferredDeletion) {
-                mDispatchedDeferredDeletion = true;
-                nsCycleCollector_dispatchDeferredDeletion();
-            }
+            SnowWhiteKiller::Visit(aBuffer, aEntry);
             return;
         }
         void *o = aEntry->mObject;
         nsCycleCollectionParticipant *cp = aEntry->mParticipant;
         CanonicalizeParticipant(&o, &cp);
         if (aEntry->mRefCnt->IsPurple() && !cp->CanSkip(o, false) &&
             (!mRemoveChildlessNodes || MayHaveChild(o, cp))) {
             return;
         }
         aBuffer.Remove(aEntry);
     }
 
 private:
     bool mRemoveChildlessNodes;
-    bool mAsyncSnowWhiteFreeing;
-    bool mDispatchedDeferredDeletion;
     CC_ForgetSkippableCallback mCallback;
 };
 
 void
-nsPurpleBuffer::RemoveSkippable(bool aRemoveChildlessNodes,
-                                bool aAsyncSnowWhiteFreeing,
+nsPurpleBuffer::RemoveSkippable(bool removeChildlessNodes,
                                 CC_ForgetSkippableCallback aCb)
 {
-    RemoveSkippableVisitor visitor(Count(), aRemoveChildlessNodes,
-                                   aAsyncSnowWhiteFreeing, aCb);
+    RemoveSkippableVisitor visitor(Count(), removeChildlessNodes, aCb);
     VisitEntries(visitor);
+    // If we're about to delete some objects when visitor goes out of scope,
+    // try to delete some more soon.
+    if (visitor.HasSnowWhiteObjects()) {
+        nsCycleCollector_dispatchDeferredDeletion();
+    }
 }
 
-bool
+class AsyncFreeSnowWhite : public nsRunnable
+{
+public:
+  NS_IMETHOD Run()
+  {
+      nsCycleCollector::TryToFreeSnowWhite();
+      return NS_OK;
+  }
+
+  static void Dispatch()
+  {
+      nsRefPtr<AsyncFreeSnowWhite> ev = new AsyncFreeSnowWhite();
+      NS_DispatchToCurrentThread(ev);
+  }
+};
+
+void
 nsCycleCollector::FreeSnowWhite(bool aUntilNoSWInPurpleBuffer)
 {
-    bool hadSnowWhiteObjects = false;
     do {
         SnowWhiteKiller visitor(mPurpleBuf.Count());
         mPurpleBuf.VisitEntries(visitor);
-        hadSnowWhiteObjects = hadSnowWhiteObjects ||
-                              visitor.HasSnowWhiteObjects();
         if (!visitor.HasSnowWhiteObjects()) {
             break;
         }
     } while (aUntilNoSWInPurpleBuffer);
-    return hadSnowWhiteObjects;
 }
 
-/* static */ bool
+/* static */ void
 nsCycleCollector::TryToFreeSnowWhite()
 {
   CollectorData* data = sCollectorData.get();
-  return data->mCollector ?
-      data->mCollector->FreeSnowWhite(false) :
-      false;
+  if (data->mCollector) {
+      data->mCollector->FreeSnowWhite(false);
+  }
 }
 
 void
 nsCycleCollector::SelectPurple(GCGraphBuilder &builder)
 {
     mPurpleBuf.SelectPointers(builder);
 }
 
 void
-nsCycleCollector::ForgetSkippable(bool aRemoveChildlessNodes,
-                                  bool aAsyncSnowWhiteFreeing)
+nsCycleCollector::ForgetSkippable(bool removeChildlessNodes)
 {
     if (mJSRuntime) {
         mJSRuntime->PrepareForForgetSkippable();
     }
-    mPurpleBuf.RemoveSkippable(aRemoveChildlessNodes, aAsyncSnowWhiteFreeing,
-                               mForgetSkippableCB);
+    mPurpleBuf.RemoveSkippable(removeChildlessNodes, mForgetSkippableCB);
 }
 
 MOZ_NEVER_INLINE void
 nsCycleCollector::MarkRoots(GCGraphBuilder &builder)
 {
     mGraph.mRootCount = builder.Count();
 
     // read the PtrInfo out of the graph that we are building
@@ -3282,29 +3252,27 @@ nsCycleCollector_setForgetSkippableCallb
     // We should have started the cycle collector by now.
     MOZ_ASSERT(data);
     MOZ_ASSERT(data->mCollector);
 
     data->mCollector->SetForgetSkippableCallback(aCB);
 }
 
 void
-nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes,
-                                 bool aAsyncSnowWhiteFreeing)
+nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes)
 {
     CollectorData *data = sCollectorData.get();
 
     // We should have started the cycle collector by now.
     MOZ_ASSERT(data);
     MOZ_ASSERT(data->mCollector);
 
     PROFILER_LABEL("CC", "nsCycleCollector_forgetSkippable");
     TimeLog timeLog;
-    data->mCollector->ForgetSkippable(aRemoveChildlessNodes,
-                                      aAsyncSnowWhiteFreeing);
+    data->mCollector->ForgetSkippable(aRemoveChildlessNodes);
     timeLog.Checkpoint("ForgetSkippable()");
 }
 
 void
 nsCycleCollector_dispatchDeferredDeletion()
 {
     AsyncFreeSnowWhite::Dispatch();
 }
--- a/xpcom/base/nsCycleCollector.h
+++ b/xpcom/base/nsCycleCollector.h
@@ -49,18 +49,17 @@ enum CCThreadingModel {
 nsresult nsCycleCollector_startup(CCThreadingModel aThreadingModel);
 
 typedef void (*CC_BeforeUnlinkCallback)(void);
 void nsCycleCollector_setBeforeUnlinkCallback(CC_BeforeUnlinkCallback aCB);
 
 typedef void (*CC_ForgetSkippableCallback)(void);
 void nsCycleCollector_setForgetSkippableCallback(CC_ForgetSkippableCallback aCB);
 
-void nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes = false,
-                                      bool aAsyncSnowWhiteFreeing = false);
+void nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes = false);
 
 void nsCycleCollector_dispatchDeferredDeletion();
 
 void nsCycleCollector_collect(bool aManuallyTriggered,
                               nsCycleCollectorResults *aResults,
                               nsICycleCollectorListener *aListener);
 uint32_t nsCycleCollector_suspectedCount();
 void nsCycleCollector_shutdownThreads();