From 676d2b13e554052f0a9928a6090dd786883d4367 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Sat, 27 Jul 2013 13:48:45 +0300 Subject: [PATCH] Bug 897433 - Telemetry for SnowWhite and more async SnowWhite freeing (patch v4), r=mccr8 --- dom/base/nsJSEnvironment.cpp | 19 ++- toolkit/components/telemetry/Histograms.json | 6 + xpcom/base/nsCycleCollector.cpp | 159 +++++++++++++------ xpcom/base/nsCycleCollector.h | 3 +- 4 files changed, 132 insertions(+), 55 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index 2529bb04644..0ec9bf7ffea 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -135,6 +135,8 @@ static PRLogModuleInfo* gJSDiagnostics; // 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; @@ -2502,7 +2504,9 @@ FireForgetSkippable(uint32_t aSuspected, bool aRemoveChildless) { PRTime startTime = PR_Now(); FinishAnyIncrementalGC(); - nsCycleCollector_forgetSkippable(aRemoveChildless); + bool earlyForgetSkippable = + sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS; + nsCycleCollector_forgetSkippable(aRemoveChildless, earlyForgetSkippable); sPreviousSuspectedCount = nsCycleCollector_suspectedCount(); ++sCleanupsSinceLastGC; PRTime delta = PR_Now() - startTime; @@ -2552,8 +2556,9 @@ nsJSContext::CycleCollectNow(nsICycleCollectorListener *aListener, // Run forgetSkippable synchronously to reduce the size of the CC graph. This // is particularly useful if we recently finished a GC. - if (sCleanupsSinceLastGC < 2 && aExtraForgetSkippableCalls >= 0) { - while (sCleanupsSinceLastGC < 2) { + if (sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS && + aExtraForgetSkippableCalls >= 0) { + while (sCleanupsSinceLastGC < NS_MAJOR_FORGET_SKIPPABLE_CALLS) { FireForgetSkippable(nsCycleCollector_suspectedCount(), false); ranSyncForgetSkippable = true; } @@ -3029,9 +3034,6 @@ DOMGCSliceCallback(JSRuntime *aRt, JS::GCProgress aProgress, const JS::GCDescrip // The GC has more work to do, so schedule another GC slice. if (aProgress == JS::GC_SLICE_END) { - if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) { - nsCycleCollector_dispatchDeferredDeletion(); - } nsJSContext::KillInterSliceGCTimer(); if (!sShuttingDown) { CallCreateInstance("@mozilla.org/timer;1", &sInterSliceGCTimer); @@ -3073,6 +3075,11 @@ DOMGCSliceCallback(JSRuntime *aRt, JS::GCProgress aProgress, const JS::GCDescrip } } + if ((aProgress == JS::GC_SLICE_END || aProgress == JS::GC_CYCLE_END) && + ShouldTriggerCC(nsCycleCollector_suspectedCount())) { + nsCycleCollector_dispatchDeferredDeletion(); + } + if (sPrevGCSliceCallback) (*sPrevGCSliceCallback)(aRt, aProgress, aDesc); } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index dd21e5edda0..42ff3583590 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -86,6 +86,12 @@ "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", diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 89b9f494c20..d8222ab02a1 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -658,6 +658,8 @@ CanonicalizeParticipant(void **parti, nsCycleCollectionParticipant **cp) } } +class nsCycleCollector; + struct nsPurpleBuffer { private: @@ -775,14 +777,16 @@ public: void SelectPointers(GCGraphBuilder &builder); - // 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, + // 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. + void RemoveSkippable(nsCycleCollector* aCollector, + bool removeChildlessNodes, + bool aAsyncSnowWhiteFreeing, CC_ForgetSkippableCallback aCb); nsPurpleBufferEntry* NewEntry() @@ -974,6 +978,8 @@ public: // Top level structure for the cycle collector. //////////////////////////////////////////////////////////////////////// +class AsyncFreeSnowWhite; + class nsCycleCollector { friend class GCGraphBuilder; @@ -995,6 +1001,8 @@ public: nsCycleCollectorParams mParams; private: + nsRefPtr mAsyncSnowWhiteFreer; + nsTArray *mWhiteNodes; uint32_t mWhiteNodeCount; @@ -1037,7 +1045,7 @@ public: void ScanRoots(); void ScanWeakMaps(); - void ForgetSkippable(bool removeChildlessNodes); + void ForgetSkippable(bool aRemoveChildlessNodes, bool aAsyncSnowWhiteFreeing); // returns whether anything was collected bool CollectWhite(nsICycleCollectorListener *aListener); @@ -1072,11 +1080,17 @@ public: bool BeginCollection(ccType aCCType, nsICycleCollectorListener *aListener); bool FinishCollection(nsICycleCollectorListener *aListener); - void FreeSnowWhite(bool aUntilNoSWInPurpleBuffer); + AsyncFreeSnowWhite* AsyncSnowWhiteFreer() + { + return mAsyncSnowWhiteFreer; + } + + bool FreeSnowWhite(bool aUntilNoSWInPurpleBuffer); // If there is a cycle collector available in the current thread, - // this calls FreeSnowWhite(false). - static void TryToFreeSnowWhite(); + // this calls FreeSnowWhite(false). Returns true if some + // snow-white objects were found. + static bool TryToFreeSnowWhite(); uint32_t SuspectedCount(); void Shutdown(); @@ -2163,6 +2177,44 @@ MayHaveChild(void *o, nsCycleCollectionParticipant* cp) return cf.MayHaveChild(); } +class AsyncFreeSnowWhite : public nsRunnable +{ +public: + NS_IMETHOD Run() + { + TimeStamp start = TimeStamp::Now(); + bool hadSnowWhiteObjects = nsCycleCollector::TryToFreeSnowWhite(); + Telemetry::Accumulate(Telemetry::CYCLE_COLLECTOR_ASYNC_SNOW_WHITE_FREEING, + uint32_t((TimeStamp::Now() - start).ToMilliseconds())); + if (hadSnowWhiteObjects && !mContinuation) { + mContinuation = true; + if (NS_FAILED(NS_DispatchToCurrentThread(this))) { + mActive = false; + } + } else { + mActive = false; + } + return NS_OK; + } + + static void Dispatch(nsCycleCollector* aCollector, bool aContinuation = false) + { + AsyncFreeSnowWhite* swf = aCollector->AsyncSnowWhiteFreer(); + if (swf->mContinuation) { + swf->mContinuation = aContinuation; + } + if (!swf->mActive && NS_SUCCEEDED(NS_DispatchToCurrentThread(swf))) { + swf->mActive = true; + } + } + + AsyncFreeSnowWhite() : mContinuation(false), mActive(false) {} + +public: + bool mContinuation; + bool mActive; +}; + struct SnowWhiteObject { void* mPointer; @@ -2223,10 +2275,15 @@ private: class RemoveSkippableVisitor : public SnowWhiteKiller { public: - RemoveSkippableVisitor(uint32_t aMaxCount, bool aRemoveChildlessNodes, + RemoveSkippableVisitor(nsCycleCollector* aCollector, + uint32_t aMaxCount, bool aRemoveChildlessNodes, + bool aAsyncSnowWhiteFreeing, CC_ForgetSkippableCallback aCb) - : SnowWhiteKiller(aMaxCount), + : SnowWhiteKiller(aAsyncSnowWhiteFreeing ? 0 : aMaxCount), + mCollector(aCollector), mRemoveChildlessNodes(aRemoveChildlessNodes), + mAsyncSnowWhiteFreeing(aAsyncSnowWhiteFreeing), + mDispatchedDeferredDeletion(false), mCallback(aCb) {} @@ -2237,6 +2294,10 @@ public: if (mCallback) { mCallback(); } + if (HasSnowWhiteObjects()) { + // Effectively a continuation. + AsyncFreeSnowWhite::Dispatch(mCollector, true); + } } void @@ -2244,7 +2305,12 @@ public: { MOZ_ASSERT(aEntry->mObject, "null mObject in purple buffer"); if (!aEntry->mRefCnt->get()) { - SnowWhiteKiller::Visit(aBuffer, aEntry); + if (!mAsyncSnowWhiteFreeing) { + SnowWhiteKiller::Visit(aBuffer, aEntry); + } else if (!mDispatchedDeferredDeletion) { + mDispatchedDeferredDeletion = true; + nsCycleCollector_dispatchDeferredDeletion(); + } return; } void *o = aEntry->mObject; @@ -2258,58 +2324,47 @@ public: } private: + nsCycleCollector* mCollector; bool mRemoveChildlessNodes; + bool mAsyncSnowWhiteFreeing; + bool mDispatchedDeferredDeletion; CC_ForgetSkippableCallback mCallback; }; void -nsPurpleBuffer::RemoveSkippable(bool removeChildlessNodes, +nsPurpleBuffer::RemoveSkippable(nsCycleCollector* aCollector, + bool aRemoveChildlessNodes, + bool aAsyncSnowWhiteFreeing, CC_ForgetSkippableCallback aCb) { - RemoveSkippableVisitor visitor(Count(), removeChildlessNodes, aCb); + RemoveSkippableVisitor visitor(aCollector, Count(), aRemoveChildlessNodes, + aAsyncSnowWhiteFreeing, 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(); - } } -class AsyncFreeSnowWhite : public nsRunnable -{ -public: - NS_IMETHOD Run() - { - nsCycleCollector::TryToFreeSnowWhite(); - return NS_OK; - } - - static void Dispatch() - { - nsRefPtr ev = new AsyncFreeSnowWhite(); - NS_DispatchToCurrentThread(ev); - } -}; - -void +bool 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 */ void +/* static */ bool nsCycleCollector::TryToFreeSnowWhite() { CollectorData* data = sCollectorData.get(); - if (data->mCollector) { - data->mCollector->FreeSnowWhite(false); - } + return data->mCollector ? + data->mCollector->FreeSnowWhite(false) : + false; } void @@ -2319,12 +2374,14 @@ nsCycleCollector::SelectPurple(GCGraphBuilder &builder) } void -nsCycleCollector::ForgetSkippable(bool removeChildlessNodes) +nsCycleCollector::ForgetSkippable(bool aRemoveChildlessNodes, + bool aAsyncSnowWhiteFreeing) { if (mJSRuntime) { mJSRuntime->PrepareForForgetSkippable(); } - mPurpleBuf.RemoveSkippable(removeChildlessNodes, mForgetSkippableCB); + mPurpleBuf.RemoveSkippable(this, aRemoveChildlessNodes, + aAsyncSnowWhiteFreeing, mForgetSkippableCB); } MOZ_NEVER_INLINE void @@ -2651,6 +2708,7 @@ nsCycleCollector::nsCycleCollector(CCThreadingModel aModel) : mJSRuntime(nullptr), mRunner(nullptr), mThread(PR_GetCurrentThread()), + mAsyncSnowWhiteFreer(new AsyncFreeSnowWhite()), mWhiteNodes(nullptr), mWhiteNodeCount(0), mVisitedRefCounted(0), @@ -3272,7 +3330,8 @@ nsCycleCollector_setForgetSkippableCallback(CC_ForgetSkippableCallback aCB) } void -nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes) +nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes, + bool aAsyncSnowWhiteFreeing) { CollectorData *data = sCollectorData.get(); @@ -3282,14 +3341,18 @@ nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes) PROFILER_LABEL("CC", "nsCycleCollector_forgetSkippable"); TimeLog timeLog; - data->mCollector->ForgetSkippable(aRemoveChildlessNodes); + data->mCollector->ForgetSkippable(aRemoveChildlessNodes, + aAsyncSnowWhiteFreeing); timeLog.Checkpoint("ForgetSkippable()"); } void nsCycleCollector_dispatchDeferredDeletion() { - AsyncFreeSnowWhite::Dispatch(); + CollectorData* data = sCollectorData.get(); + if (data && data->mCollector) { + AsyncFreeSnowWhite::Dispatch(data->mCollector); + } } void diff --git a/xpcom/base/nsCycleCollector.h b/xpcom/base/nsCycleCollector.h index 2773492a782..23571c73da8 100644 --- a/xpcom/base/nsCycleCollector.h +++ b/xpcom/base/nsCycleCollector.h @@ -54,7 +54,8 @@ void nsCycleCollector_setBeforeUnlinkCallback(CC_BeforeUnlinkCallback aCB); typedef void (*CC_ForgetSkippableCallback)(void); void nsCycleCollector_setForgetSkippableCallback(CC_ForgetSkippableCallback aCB); -void nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes = false); +void nsCycleCollector_forgetSkippable(bool aRemoveChildlessNodes = false, + bool aAsyncSnowWhiteFreeing = false); void nsCycleCollector_dispatchDeferredDeletion();