863085 - Remove NS_CycleCollectorForget2 and friends

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

Thanks to Olli's ref counting improvements the cycle collector never forgets, so we should just get rid of this function.  See bug 785666 for the changes that may be required for the xpcom/{stub, build} stuff.

Comment 1

[Andrew McCreight [:mccr8]]

Looks like xpcom/stub is gone.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 1: gut NS_CycleCollectorForget2 and rename nsCycleCollector::Suspect2 to Suspect

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 743751 [details] [diff] [review]
part 1: gut NS_CycleCollectorForget2 and rename nsCycleCollector::Suspect2 to Suspect

># HG changeset patch
># User Andrew McCreight <[e-mail]>
># Date 1367345699 25200
># Node ID a6d1e489f366523583d6365ec5608ada430ffcf9
># Parent  122a97fd3d4d8a99ffa5287c0cf49c9d0ff6f9da
>Bug 863085, part 1 - Gut NS_CycleCollectorForget2 and rename CC:Suspect2. r=smaug
>
>diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp
>--- a/xpcom/base/nsCycleCollector.cpp
>+++ b/xpcom/base/nsCycleCollector.cpp
>@@ -1064,18 +1064,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 ShutdownCollect(nsICycleCollectorListener *aListener);
> 
> public:
>     void Collect(ccType aCCType,
>@@ -2596,17 +2595,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)
>@@ -2617,33 +2616,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
> }
> 
>@@ -2990,34 +2972,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) {

Comment 4

[Andrew McCreight [:mccr8]]

I should also update the comments in nsCycleCollector.cpp which talk about forget and are horribly outdated at this point.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: part 2: remove some bits of supect2 from XPCOM glue

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: part 1: gut NS_CycleCollectorForget2 and rename nsCycleCollector::Suspect2 to Suspect

I updated the comments in the file so they are not quite so out-of-date.

Comment 7

[Andrew McCreight [:mccr8]]

try run: https://tbpl.mozilla.org/?tree=Try&rev=0e536afd216b

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 743888 [details] [diff] [review]
part 1: gut NS_CycleCollectorForget2 and rename nsCycleCollector::Suspect2 to Suspect

So we have empty NS_CycleCollectorForget2?
Could we at least add a warning if one tries to use it.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Oh, the other patch removes it.

Comment 10

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/210b52cdd7eb
https://hg.mozilla.org/integration/mozilla-inbound/rev/5aceec8a1222

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/210b52cdd7eb
https://hg.mozilla.org/mozilla-central/rev/5aceec8a1222