Bug 863085, part 1 - Gut NS_CycleCollectorForget2 and rename CC::Suspect2. r=smaug
authorAndrew McCreight <amccreight@mozilla.com>
Wed, 01 May 2013 15:35:13 -0700
changeset 141504 210b52cdd7ebf57603f51abbb2a1d5c2e7dd5b7d
parent 141503 e5d90d0479ccd39c8f571d63300e8ecd16df7abc
child 141505 5aceec8a122219fecb8ea8900cce27c2b5a7086c
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs863085
milestone23.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 863085, part 1 - Gut NS_CycleCollectorForget2 and rename CC::Suspect2. r=smaug
xpcom/base/nsCycleCollector.cpp
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -40,50 +40,54 @@
 // black (incremented refcount) or are ultimately deleted.
 
 
 // Safety:
 //
 // An XPCOM object is either scan-safe or scan-unsafe, purple-safe or
 // purple-unsafe.
 //
-// An object is scan-safe if:
+// An nsISupports object is scan-safe if:
 //
-//  - It can be QI'ed to |nsXPCOMCycleCollectionParticipant|, though this
-//    operation loses ISupports identity (like nsIClassInfo).
-//  - The operation |traverse| on the resulting
+//  - It can be QI'ed to |nsXPCOMCycleCollectionParticipant|, though
+//    this operation loses ISupports identity (like nsIClassInfo).
+//  - Additionally, the operation |traverse| on the resulting
 //    nsXPCOMCycleCollectionParticipant does not cause *any* refcount
 //    adjustment to occur (no AddRef / Release calls).
 //
+// A non-nsISupports ("native") object is scan-safe by explicitly
+// providing its nsCycleCollectionParticipant.
+//
 // An object is purple-safe if it satisfies the following properties:
 //
 //  - The object is scan-safe.  
 //  - If the object calls |nsCycleCollector::suspect(this)|, 
-//    it will eventually call |nsCycleCollector::forget(this)|, 
-//    exactly once per call to |suspect|, before being destroyed.
+//    it will null out the pointer from the purple buffer entry to
+//    the object before being destroyed.
 //
 // When we receive a pointer |ptr| via
 // |nsCycleCollector::suspect(ptr)|, we assume it is purple-safe. We
 // can check the scan-safety, but have no way to ensure the
 // purple-safety; objects must obey, or else the entire system falls
 // apart. Don't involve an object in this scheme if you can't
-// guarantee its purple-safety.
+// guarantee its purple-safety. The easiest way to ensure that an
+// object is purple-safe is to use nsCycleCollectingAutoRefCnt.
 //
 // When we have a scannable set of purple nodes ready, we begin
 // our walks. During the walks, the nodes we |traverse| should only
 // feed us more scan-safe nodes, and should not adjust the refcounts
 // of those nodes. 
 //
 // We do not |AddRef| or |Release| any objects during scanning. We
-// rely on purple-safety of the roots that call |suspect| and
-// |forget| to hold, such that we will forget about a purple pointer
-// before it is destroyed.  The pointers that are merely scan-safe,
-// we hold only for the duration of scanning, and there should be no
-// objects released from the scan-safe set during the scan (there
-// should be no threads involved).
+// rely on the purple-safety of the roots that call |suspect| to
+// hold, such that we will clear the pointer from the purple buffer
+// entry to the object before it is destroyed. The pointers that are
+// merely scan-safe we hold only for the duration of scanning, and
+// there should be no objects released from the scan-safe set during
+// the scan.
 //
 // We *do* call |AddRef| and |Release| on every white object, on
 // either side of the calls to |Unlink|. This keeps the set of white
 // objects alive during the unlinking.
 // 
 
 #if !defined(__MINGW32__)
 #ifdef WIN32
@@ -1056,18 +1060,17 @@ public:
     bool CollectWhite(nsICycleCollectorListener *aListener);
 
     nsCycleCollector(CCThreadingModel aModel);
     ~nsCycleCollector();
 
     nsresult Init();
     void ShutdownThreads();
 
-    nsPurpleBufferEntry* Suspect2(void *n, nsCycleCollectionParticipant *cp);
-    bool Forget2(nsPurpleBufferEntry *e);
+    nsPurpleBufferEntry* Suspect(void *n, nsCycleCollectionParticipant *cp);
 
     void CheckThreadSafety();
 
 private:
     void MainThreadCollect(bool aMergeZones,
                            nsCycleCollectorResults *aResults,
                            uint32_t aTryCollections,
                            nsICycleCollectorListener *aListener);
@@ -2588,17 +2591,17 @@ nsCycleCollector_isScanSafe(void *s, nsC
     nsXPCOMCycleCollectionParticipant *xcp;
     ToParticipant(static_cast<nsISupports*>(s), &xcp);
 
     return xcp != nullptr;
 }
 #endif
 
 nsPurpleBufferEntry*
-nsCycleCollector::Suspect2(void *n, nsCycleCollectionParticipant *cp)
+nsCycleCollector::Suspect(void *n, nsCycleCollectionParticipant *cp)
 {
     CheckThreadSafety();
 
     // Re-entering ::Suspect during collection used to be a fault, but
     // we are canonicalizing nsISupports pointers using QI, so we will
     // see some spurious refcount traffic here. 
 
     if (mScanInProgress)
@@ -2609,33 +2612,16 @@ nsCycleCollector::Suspect2(void *n, nsCy
 
     if (mParams.mDoNothing)
         return nullptr;
 
     // Caller is responsible for filling in result's mRefCnt.
     return mPurpleBuf.Put(n, cp);
 }
 
-
-bool
-nsCycleCollector::Forget2(nsPurpleBufferEntry *e)
-{
-    CheckThreadSafety();
-
-    // Re-entering ::Forget during collection used to be a fault, but
-    // we are canonicalizing nsISupports pointers using QI, so we will
-    // see some spurious refcount traffic here. 
-
-    if (mScanInProgress)
-        return false;
-
-    mPurpleBuf.Remove(e);
-    return true;
-}
-
 void
 nsCycleCollector::CheckThreadSafety()
 {
 #ifdef DEBUG
     MOZ_ASSERT(mThread == PR_GetCurrentThread());
 #endif
 }
 
@@ -2949,34 +2935,23 @@ NS_CycleCollectorSuspect2(void *n, nsCyc
     }
 
     if (collector == (nsCycleCollector*)1) {
         // This is our special sentinel value that tells us that we've shut
         // down this thread's CC.
         return nullptr;
     }
 
-    return collector->Suspect2(n, cp);
+    return collector->Suspect(n, cp);
 }
 
 bool
 NS_CycleCollectorForget2(nsPurpleBufferEntry *e)
 {
-    nsCycleCollector *collector = sCollector.get();
-
-    if (!collector)
-        MOZ_CRASH();
-
-    if (collector == (nsCycleCollector*)1) {
-        // This is our special sentinel value that tells us that we've shut
-        // down this thread's CC.
-        return true;
-    }
-
-    return collector->Forget2(e);
+    return true;
 }
 
 uint32_t
 nsCycleCollector_suspectedCount()
 {
     nsCycleCollector *collector = sCollector.get();
 
     if (collector == (nsCycleCollector*)1) {