Issue #2678 - Don't use QI to canonicalize nsISupports pointers in the purple buffer.

The nsISupports objects added to the purple buffer are already canonical,
so we can avoid some overhead by not QIing them to
`nsCycleCollectionISupports`.
This commit is contained in:
Moonchild 2025-01-16 21:56:13 +01:00 • committed by roytam1
commit 9f4fe4275c

View file

@ -961,27 +961,6 @@ CanonicalizeXPCOMParticipant(nsISupports* aIn)
return out; return out;
} }
static inline void
ToParticipant(nsISupports* aPtr, nsXPCOMCycleCollectionParticipant** aCp);
static void
CanonicalizeParticipant(void** aParti, nsCycleCollectionParticipant** aCp)
{
// If the participant is null, this is an nsISupports participant,
// so we must QI to get the real participant.
if (!*aCp) {
nsISupports* nsparti = static_cast<nsISupports*>(*aParti);
nsparti = CanonicalizeXPCOMParticipant(nsparti);
NS_ASSERTION(nsparti,
"Don't add objects that don't participate in collection!");
nsXPCOMCycleCollectionParticipant* xcp;
ToParticipant(nsparti, &xcp);
*aParti = nsparti;
*aCp = xcp;
}
}
struct nsPurpleBufferEntry struct nsPurpleBufferEntry
{ {
nsPurpleBufferEntry(void* aObject, nsCycleCollectingAutoRefCnt* aRefCnt, nsPurpleBufferEntry(void* aObject, nsCycleCollectingAutoRefCnt* aRefCnt,
@ -1414,6 +1393,21 @@ ToParticipant(nsISupports* aPtr, nsXPCOMCycleCollectionParticipant** aCp)
CallQueryInterface(aPtr, aCp); CallQueryInterface(aPtr, aCp);
} }
static void
ToParticipant(void* aParti, nsCycleCollectionParticipant** aCp)
{
// If the participant is null, this is an nsISupports participant,
// so we must QI to get the real participant.
if (!*aCp) {
nsISupports* nsparti = static_cast<nsISupports*>(aParti);
MOZ_ASSERT(CanonicalizeXPCOMParticipant(nsparti) == nsparti);
nsXPCOMCycleCollectionParticipant* xcp;
ToParticipant(nsparti, &xcp);
*aCp = xcp;
}
}
template<class Visitor> template<class Visitor>
MOZ_NEVER_INLINE void MOZ_NEVER_INLINE void
GraphWalker<Visitor>::Walk(PtrInfo* aPi) GraphWalker<Visitor>::Walk(PtrInfo* aPi)
@ -2220,7 +2214,7 @@ CCGraphBuilder::AddNode(void* aPtr, nsCycleCollectionParticipant* aParticipant)
bool bool
CCGraphBuilder::AddPurpleRoot(void* aRoot, nsCycleCollectionParticipant* aParti) CCGraphBuilder::AddPurpleRoot(void* aRoot, nsCycleCollectionParticipant* aParti)
{ {
CanonicalizeParticipant(&aRoot, &aParti); ToParticipant(aRoot, &aParti);
if (WantAllTraces() || !aParti->CanSkipInCC(aRoot)) { if (WantAllTraces() || !aParti->CanSkipInCC(aRoot)) {
PtrInfo* pinfo = AddNode(aRoot, aParti); PtrInfo* pinfo = AddNode(aRoot, aParti);
@ -2658,7 +2652,7 @@ public:
if (!aEntry->mRefCnt->get()) { if (!aEntry->mRefCnt->get()) {
void* o = aEntry->mObject; void* o = aEntry->mObject;
nsCycleCollectionParticipant* cp = aEntry->mParticipant; nsCycleCollectionParticipant* cp = aEntry->mParticipant;
CanonicalizeParticipant(&o, &cp); ToParticipant(o, &cp);
SnowWhiteObject swo = { o, cp, aEntry->mRefCnt }; SnowWhiteObject swo = { o, cp, aEntry->mRefCnt };
mObjects.InfallibleAppend(swo); mObjects.InfallibleAppend(swo);
aBuffer.Remove(aEntry); aBuffer.Remove(aEntry);
@ -2774,7 +2768,7 @@ public:
} }
void* o = aEntry->mObject; void* o = aEntry->mObject;
nsCycleCollectionParticipant* cp = aEntry->mParticipant; nsCycleCollectionParticipant* cp = aEntry->mParticipant;
CanonicalizeParticipant(&o, &cp); ToParticipant(o, &cp);
if (aEntry->mRefCnt->IsPurple() && !cp->CanSkip(o, false) && if (aEntry->mRefCnt->IsPurple() && !cp->CanSkip(o, false) &&
(!mRemoveChildlessNodes || MayHaveChild(o, cp))) { (!mRemoveChildlessNodes || MayHaveChild(o, cp))) {
return; return;
@ -2981,10 +2975,8 @@ public:
"Snow-white objects shouldn't be in the purple buffer."); "Snow-white objects shouldn't be in the purple buffer.");
void* obj = aEntry->mObject; void* obj = aEntry->mObject;
if (!aEntry->mParticipant) { MOZ_ASSERT(aEntry->mParticipant || CanonicalizeXPCOMParticipant(static_cast<nsISupports*>(obj)) == obj,
obj = CanonicalizeXPCOMParticipant(static_cast<nsISupports*>(obj)); "Suspect nsISupports pointer must be canonical");
MOZ_ASSERT(obj, "Don't add objects that don't participate in collection!");
}
PtrInfo* pi = mGraph.FindNode(obj); PtrInfo* pi = mGraph.FindNode(obj);
if (!pi) { if (!pi) {
@ -3455,6 +3447,9 @@ nsCycleCollector::Suspect(void* aPtr, nsCycleCollectionParticipant* aParti,
MOZ_ASSERT(HasParticipant(aPtr, aParti), MOZ_ASSERT(HasParticipant(aPtr, aParti),
"Suspected nsISupports pointer must QI to nsXPCOMCycleCollectionParticipant"); "Suspected nsISupports pointer must QI to nsXPCOMCycleCollectionParticipant");
MOZ_ASSERT(aParti || CanonicalizeXPCOMParticipant(static_cast<nsISupports*>(aPtr)) == aPtr,
"Suspect nsISupports pointer must be canonical");
mPurpleBuf.Put(aPtr, aParti, aRefCnt); mPurpleBuf.Put(aPtr, aParti, aRefCnt);
} }
@ -3953,7 +3948,7 @@ SuspectAfterShutdown(void* aPtr, nsCycleCollectionParticipant* aCp,
if (aRefCnt->get() == 0) { if (aRefCnt->get() == 0) {
if (!aShouldDelete) { if (!aShouldDelete) {
// The CC is shut down, so we can't be in the middle of an ICC. // The CC is shut down, so we can't be in the middle of an ICC.
CanonicalizeParticipant(&aPtr, &aCp); ToParticipant(aPtr, &aCp);
aRefCnt->stabilizeForDeletion(); aRefCnt->stabilizeForDeletion();
aCp->DeleteCycleCollectable(aPtr); aCp->DeleteCycleCollectable(aPtr);
} else { } else {