From a77ba18be990ac28feecb7e8bd76103655ba5828 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Wed, 6 May 2015 20:51:00 +0200 Subject: [PATCH] Bug 1154053 - Limit concurrency of e10s memory reporting. r=erahm This changes the way nsMemoryReporterManger handles child processes; instead of using an observer message and trying to keep a count of child processes expected to answer, it directly iterates a copy of the list of content processes and explicitly handles children which exit before their reports start. Note that GC/CC logs still run at full concurrency, and that no child reports start until the parent is finished (see bug 1151597) regardless of concurrency limit. --- dom/ipc/ContentParent.cpp | 73 +------ modules/libpref/init/all.js | 10 + .../aboutmemory/tests/test_aboutmemory5.xul | 1 + xpcom/base/nsMemoryReporterManager.cpp | 181 ++++++++++-------- xpcom/base/nsMemoryReporterManager.h | 68 ++++--- 5 files changed, 160 insertions(+), 173 deletions(-) diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 4dacfa250a4f..1bdc40a7e562 100755 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -424,7 +424,7 @@ public: private: const uint32_t mGeneration; - // Non-null if we haven't yet called EndChildReport() on it. + // Non-null if we haven't yet called EndProcessReport() on it. nsRefPtr mReporterManager; ContentParent* Owner() @@ -464,7 +464,7 @@ void MemoryReportRequestParent::ActorDestroy(ActorDestroyReason aWhy) { if (mReporterManager) { - mReporterManager->EndChildReport(mGeneration, aWhy == Deletion); + mReporterManager->EndProcessReport(mGeneration, aWhy == Deletion); mReporterManager = nullptr; } } @@ -648,7 +648,6 @@ static const char* sObserverTopics[] = { "profile-before-change", NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC, NS_IPC_IOSERVICE_SET_CONNECTIVITY_TOPIC, - "child-memory-reporter-request", "memory-pressure", "child-gc-request", "child-cc-request", @@ -1976,18 +1975,6 @@ ContentParent::ActorDestroy(ActorDestroyReason why) } } - // Tell the memory reporter manager that this ContentParent is going away. - nsRefPtr mgr = - nsMemoryReporterManager::GetOrCreate(); -#ifdef MOZ_NUWA_PROCESS - bool isMemoryChild = !IsNuwaProcess(); -#else - bool isMemoryChild = true; -#endif - if (mgr && isMemoryChild) { - mgr->DecrementNumChildProcesses(); - } - // remove the global remote preferences observers Preferences::RemoveObserver(this, ""); @@ -2261,15 +2248,6 @@ ContentParent::ContentParent(mozIApplication* aApp, IToplevelProtocol::SetTransport(mSubprocess->GetChannel()); - if (!aIsNuwaProcess) { - // Tell the memory reporter manager that this ContentParent exists. - nsRefPtr mgr = - nsMemoryReporterManager::GetOrCreate(); - if (mgr) { - mgr->IncrementNumChildProcesses(); - } - } - std::vector extraArgs; if (aIsNuwaProcess) { extraArgs.push_back("-nuwa"); @@ -2334,13 +2312,6 @@ ContentParent::ContentParent(ContentParent* aTemplate, aPid, *fd); - // Tell the memory reporter manager that this ContentParent exists. - nsRefPtr mgr = - nsMemoryReporterManager::GetOrCreate(); - if (mgr) { - mgr->IncrementNumChildProcesses(); - } - mSubprocess->LaunchAndWaitForProcessHandle(); // Clone actors routed by aTemplate for this instance. @@ -3062,46 +3033,6 @@ ContentParent::Observe(nsISupports* aSubject, nsDependentString(aData))) return NS_ERROR_NOT_AVAILABLE; } - else if (!strcmp(aTopic, "child-memory-reporter-request")) { - bool isNuwa = false; -#ifdef MOZ_NUWA_PROCESS - isNuwa = IsNuwaProcess(); -#endif - if (!isNuwa) { - unsigned generation; - int anonymize, minimize, identOffset = -1; - nsDependentString msg(aData); - NS_ConvertUTF16toUTF8 cmsg(msg); - - if (sscanf(cmsg.get(), - "generation=%x anonymize=%d minimize=%d DMDident=%n", - &generation, &anonymize, &minimize, &identOffset) < 3 - || identOffset < 0) { - return NS_ERROR_INVALID_ARG; - } - // The pre-%n part of the string should be all ASCII, so the byte - // offset in identOffset should be correct as a char offset. - MOZ_ASSERT(cmsg[identOffset - 1] == '='); - MaybeFileDesc dmdFileDesc = void_t(); -#ifdef MOZ_DMD - nsAutoString dmdIdent(Substring(msg, identOffset)); - if (!dmdIdent.IsEmpty()) { - FILE *dmdFile = nullptr; - nsresult rv = nsMemoryInfoDumper::OpenDMDFile(dmdIdent, Pid(), &dmdFile); - if (NS_WARN_IF(NS_FAILED(rv))) { - // Proceed with the memory report as if DMD were disabled. - dmdFile = nullptr; - } - if (dmdFile) { - dmdFileDesc = FILEToFileDescriptor(dmdFile); - fclose(dmdFile); - } - } -#endif - unused << SendPMemoryReportRequestConstructor( - generation, anonymize, minimize, dmdFileDesc); - } - } else if (!strcmp(aTopic, "child-gc-request")){ unused << SendGarbageCollect(); } diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 897bb1c3ff67..ea8a3ca5ca74 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4749,3 +4749,13 @@ pref("dom.secureelement.enabled", false); // both composition string and data attribute of compositionupdate // and compositionend events. pref("dom.compositionevent.allow_control_characters", false); + +#ifdef MOZ_WIDGET_GONK +// Bug 1154053: Serialize B2G memory reports; smaller devices are +// usually overcommitted on memory by using zRAM, so memory reporting +// causes memory pressure from uncompressing cold heap memory. +pref("memory.report_concurrency", 1); +#else +// Desktop probably doesn't have swapped-out children like that. +pref("memory.report_concurrency", 10); +#endif diff --git a/toolkit/components/aboutmemory/tests/test_aboutmemory5.xul b/toolkit/components/aboutmemory/tests/test_aboutmemory5.xul index 871e75b13c92..2fec803b9cb4 100644 --- a/toolkit/components/aboutmemory/tests/test_aboutmemory5.xul +++ b/toolkit/components/aboutmemory/tests/test_aboutmemory5.xul @@ -35,6 +35,7 @@ let prefs = [ ["dom.ipc.processCount", 3], // Allow up to 3 child processes + ["memory.report_concurrency", 2], // Cover more child handling cases ["memory.system_memory_reporter", true] // Test SystemMemoryReporter ]; diff --git a/xpcom/base/nsMemoryReporterManager.cpp b/xpcom/base/nsMemoryReporterManager.cpp index aaaf87550eba..5905ff71e756 100644 --- a/xpcom/base/nsMemoryReporterManager.cpp +++ b/xpcom/base/nsMemoryReporterManager.cpp @@ -23,9 +23,12 @@ #endif #include "mozilla/Attributes.h" #include "mozilla/PodOperations.h" +#include "mozilla/Preferences.h" #include "mozilla/Services.h" #include "mozilla/Telemetry.h" #include "mozilla/dom/PMemoryReportRequestParent.h" // for dom::MemoryReport +#include "mozilla/dom/ContentParent.h" +#include "mozilla/ipc/FileDescriptorUtils.h" #ifdef XP_WIN #include @@ -1246,7 +1249,6 @@ nsMemoryReporterManager::nsMemoryReporterManager() , mWeakReporters(new WeakReportersTable()) , mSavedStrongReporters(nullptr) , mSavedWeakReporters(nullptr) - , mNumChildProcesses(0) , mNextGeneration(1) , mGetReportsState(nullptr) { @@ -1271,29 +1273,6 @@ nsMemoryReporterManager::~nsMemoryReporterManager() #define MEMORY_REPORTING_LOG(...) #endif -void -nsMemoryReporterManager::IncrementNumChildProcesses() -{ - if (!NS_IsMainThread()) { - MOZ_CRASH(); - } - mNumChildProcesses++; - MEMORY_REPORTING_LOG("IncrementNumChildProcesses --> %d\n", - mNumChildProcesses); -} - -void -nsMemoryReporterManager::DecrementNumChildProcesses() -{ - if (!NS_IsMainThread()) { - MOZ_CRASH(); - } - MOZ_ASSERT(mNumChildProcesses > 0); - mNumChildProcesses--; - MEMORY_REPORTING_LOG("DecrementNumChildProcesses --> %d\n", - mNumChildProcesses); -} - NS_IMETHODIMP nsMemoryReporterManager::GetReports( nsIHandleReportCallback* aHandleReport, @@ -1337,18 +1316,23 @@ nsMemoryReporterManager::GetReportsExtended( return NS_OK; } - MEMORY_REPORTING_LOG("GetReports (gen=%u, %d child(ren) present)\n", - generation, mNumChildProcesses); + MEMORY_REPORTING_LOG("GetReports (gen=%u)\n", generation); + uint32_t concurrency = Preferences::GetUint("memory.report_concurrency", 1); + MOZ_ASSERT(concurrency >= 1); + if (concurrency < 1) { + concurrency = 1; + } mGetReportsState = new GetReportsState(generation, aAnonymize, aMinimize, - mNumChildProcesses, + concurrency, aHandleReport, aHandleReportData, aFinishReporting, aFinishReportingData, aDMDDumpIdent); + mGetReportsState->mChildrenPending = new nsTArray>(); if (aMinimize) { rv = MinimizeMemoryUsage(NS_NewRunnableMethod( @@ -1380,12 +1364,18 @@ nsMemoryReporterManager::StartGettingReports() GetReportsForThisProcessExtended(s->mHandleReport, s->mHandleReportData, s->mAnonymize, parentDMDFile); - MOZ_ASSERT(s->mNumChildProcessesCompleted == 0); - if (s->mNumChildProcesses > 0) { + nsTArray childWeakRefs; + ContentParent::GetAll(childWeakRefs); + if (!childWeakRefs.IsEmpty()) { // Request memory reports from child processes. This happens // after the parent report so that the parent's main thread will // be free to process the child reports, instead of causing them // to be buffered and consume (possibly scarce) memory. + + for (size_t i = 0; i < childWeakRefs.Length(); ++i) { + s->mChildrenPending->AppendElement(childWeakRefs[i]); + } + nsCOMPtr timer = do_CreateInstance(NS_TIMER_CONTRACTID); // Don't use NS_ENSURE_* here; can't return until the report is finished. if (NS_WARN_IF(!timer)) { @@ -1402,27 +1392,12 @@ nsMemoryReporterManager::StartGettingReports() MOZ_ASSERT(!s->mTimer); s->mTimer.swap(timer); - - nsCOMPtr obs = services::GetObserverService(); - if (NS_WARN_IF(!obs)) { - FinishReporting(); - return NS_ERROR_UNEXPECTED; - } - - nsPrintfCString genStr("generation=%x anonymize=%d minimize=%d DMDident=", - s->mGeneration, s->mAnonymize ? 1 : 0, - s->mMinimize ? 1 : 0); - nsAutoString msg = NS_ConvertUTF8toUTF16(genStr); - msg += s->mDMDDumpIdent; - - obs->NotifyObservers(nullptr, "child-memory-reporter-request", - msg.get()); - - return NS_OK; - } else { - // If there are no child processes, we can finish up immediately. - return FinishReporting(); } + + // The parent's report is done; make note of that, and start + // launching child process reports (if any). + EndProcessReport(s->mGeneration, true); + return NS_OK; } typedef nsCOMArray MemoryReporterArray; @@ -1502,17 +1477,12 @@ nsMemoryReporterManager::GetStateForGeneration(uint32_t aGeneration) GetReportsState* s = mGetReportsState; if (!s) { - // If we reach here, either: + // If we reach here, then: // // - A child process reported back too late, and no subsequent request // is in flight. // - // - (Unlikely) A "child-memory-reporter-request" notification was - // triggered from somewhere other than GetReports(), causing child - // processes to report back when the nsMemoryReporterManager wasn't - // expecting it. - // - // Either way, there's nothing to be done. Just ignore it. + // So there's nothing to be done. Just ignore it. MEMORY_REPORTING_LOG( "HandleChildReports: no request in flight (aGen=%u)\n", aGeneration); @@ -1559,31 +1529,85 @@ nsMemoryReporterManager::HandleChildReport( s->mHandleReportData); } +/* static */ bool +nsMemoryReporterManager::StartChildReport(mozilla::dom::ContentParent* aChild, + const GetReportsState* aState) +{ +#ifdef MOZ_NUWA_PROCESS + if (aChild->IsNuwaProcess()) { + return false; + } +#endif + + if (!aChild->IsAlive()) { + MEMORY_REPORTING_LOG("StartChildReports (gen=%u): child exited before" + " its report was started\n", + aState->mGeneration); + return false; + } + + mozilla::dom::MaybeFileDesc dmdFileDesc = void_t(); +#ifdef MOZ_DMD + if (!aState->mDMDDumpIdent.IsEmpty()) { + FILE *dmdFile = nullptr; + nsresult rv = nsMemoryInfoDumper::OpenDMDFile(aState->mDMDDumpIdent, + aChild->Pid(), &dmdFile); + if (NS_WARN_IF(NS_FAILED(rv))) { + // Proceed with the memory report as if DMD were disabled. + dmdFile = nullptr; + } + if (dmdFile) { + dmdFileDesc = mozilla::ipc::FILEToFileDescriptor(dmdFile); + fclose(dmdFile); + } + } +#endif + return aChild->SendPMemoryReportRequestConstructor( + aState->mGeneration, aState->mAnonymize, aState->mMinimize, dmdFileDesc); +} + void -nsMemoryReporterManager::EndChildReport(uint32_t aGeneration, bool aSuccess) +nsMemoryReporterManager::EndProcessReport(uint32_t aGeneration, bool aSuccess) { GetReportsState* s = GetStateForGeneration(aGeneration); if (!s) { return; } - s->mNumChildProcessesCompleted++; + MOZ_ASSERT(s->mNumProcessesRunning > 0); + s->mNumProcessesRunning--; + s->mNumProcessesCompleted++; + MEMORY_REPORTING_LOG("HandleChildReports (aGen=%u): process %u %s" + " (%u running, %u pending)\n", + aGeneration, s->mNumProcessesCompleted, + aSuccess ? "completed" : "exited during report", + s->mNumProcessesRunning, + static_cast(s->mChildrenPending->Length())); - if (aSuccess) { - MEMORY_REPORTING_LOG("HandleChildReports (aGen=%u): completed child %d\n", - aGeneration, s->mNumChildProcessesCompleted); - } else { - // Unfortunately, there's no way to indicate this in the report yet. - // (Also, we don't have the child's identifier at this point.) - MEMORY_REPORTING_LOG("HandleChildReports (aGen=%u): child %d exited" - " during report\n", - aGeneration, s->mNumChildProcessesCompleted); + // Start pending children up to the concurrency limit. + while (s->mNumProcessesRunning < s->mConcurrencyLimit && + !s->mChildrenPending->IsEmpty()) { + // Pop last element from s->mChildrenPending + nsRefPtr nextChild; + nextChild.swap(s->mChildrenPending->LastElement()); + s->mChildrenPending->TruncateLength(s->mChildrenPending->Length() - 1); + // Start report (if the child is still alive and not Nuwa). + if (StartChildReport(nextChild, s)) { + ++s->mNumProcessesRunning; + MEMORY_REPORTING_LOG("HandleChildReports (aGen=%u): started child report" + " (%u running, %u pending)\n", + aGeneration, s->mNumProcessesRunning, + static_cast(s->mChildrenPending->Length())); + } } - // If all the child processes have reported, we can cancel the timer and - // finish up. Otherwise, just return. - if (s->mNumChildProcessesCompleted >= s->mNumChildProcesses) { - s->mTimer->Cancel(); + // If all the child processes (if any) have reported, we can cancel + // the timer (if started) and finish up. Otherwise, just return. + if (s->mNumProcessesRunning == 0) { + MOZ_ASSERT(s->mChildrenPending->IsEmpty()); + if (s->mTimer) { + s->mTimer->Cancel(); + } FinishReporting(); } } @@ -1598,8 +1622,9 @@ nsMemoryReporterManager::TimeoutCallback(nsITimer* aTimer, void* aData) // crash regardless of DEBUG, and this way the compiler doesn't // complain about unused variables. MOZ_RELEASE_ASSERT(s, "mgr->mGetReportsState"); - MEMORY_REPORTING_LOG("TimeoutCallback (s->gen=%u)\n", - s->mGeneration); + MEMORY_REPORTING_LOG("TimeoutCallback (s->gen=%u; %u running, %u pending)\n", + s->mGeneration, s->mNumProcessesRunning, + static_cast(s->mChildrenPending->Length())); // We don't bother sending any kind of cancellation message to the child // processes that haven't reported back. @@ -1615,8 +1640,9 @@ nsMemoryReporterManager::FinishReporting() } MOZ_ASSERT(mGetReportsState); - MEMORY_REPORTING_LOG("FinishReporting (s->gen=%u)\n", - mGetReportsState->mGeneration); + MEMORY_REPORTING_LOG("FinishReporting (s->gen=%u; %u processes reported)\n", + mGetReportsState->mGeneration, + mGetReportsState->mNumProcessesCompleted); // Call this before deleting |mGetReportsState|. That way, if // |mFinishReportData| calls GetReports(), it will silently abort, as @@ -1629,6 +1655,11 @@ nsMemoryReporterManager::FinishReporting() return rv; } +nsMemoryReporterManager::GetReportsState::~GetReportsState() +{ + delete mChildrenPending; +} + static void CrashIfRefcountIsZero(nsISupports* aObj) { diff --git a/xpcom/base/nsMemoryReporterManager.h b/xpcom/base/nsMemoryReporterManager.h index 10f8870abc7e..e239b80a8bf4 100644 --- a/xpcom/base/nsMemoryReporterManager.h +++ b/xpcom/base/nsMemoryReporterManager.h @@ -7,21 +7,22 @@ #ifndef nsMemoryReporterManager_h__ #define nsMemoryReporterManager_h__ +#include "mozilla/Mutex.h" +#include "nsHashKeys.h" #include "nsIMemoryReporter.h" #include "nsITimer.h" #include "nsServiceManagerUtils.h" -#include "mozilla/Mutex.h" #include "nsTHashtable.h" -#include "nsHashKeys.h" - -class nsITimer; namespace mozilla { namespace dom { +class ContentParent; class MemoryReport; } } +class nsITimer; + class nsMemoryReporterManager final : public nsIMemoryReporterManager { virtual ~nsMemoryReporterManager(); @@ -43,24 +44,27 @@ public: typedef nsTHashtable> StrongReportersTable; typedef nsTHashtable> WeakReportersTable; - void IncrementNumChildProcesses(); - void DecrementNumChildProcesses(); - // Inter-process memory reporting proceeds as follows. // // - GetReports() (declared within NS_DECL_NSIMEMORYREPORTERMANAGER) - // synchronously gets memory reports for the current process, tells all - // child processes to get memory reports, and sets up some state - // (mGetReportsState) for when the child processes report back, including a - // timer. Control then returns to the main event loop. + // synchronously gets memory reports for the current process, sets up some + // state (mGetReportsState) for when child processes report back -- + // including a timer -- and starts telling child processes to get memory + // reports. Control then returns to the main event loop. + // + // The number of concurrent child process reports is limited by the pref + // "memory.report_concurrency" in order to prevent the memory overhead of + // memory reporting from causing problems, especially on B2G when swapping + // to compressed RAM; see bug 1154053. // // - HandleChildReport() is called (asynchronously) once per child process // reporter callback. // - // - EndChildReport() is called (asynchronously) once per child process that - // finishes reporting back. If all child processes do so before time-out, - // the timer is cancelled. (The number of child processes is part of the - // saved request state.) + // - EndProcessReport() is called (asynchronously) once per process that + // finishes reporting back, including the parent. If all processes do so + // before time-out, the timer is cancelled. If there are child processes + // whose requests have not yet been sent, they will be started until the + // concurrency limit is (again) reached. // // - TimeoutCallback() is called (asynchronously) if all the child processes // don't respond within the time threshold. @@ -104,12 +108,12 @@ public: // is incomplete. // // Now, what what happens if a child process is created/destroyed in the - // middle of a request? Well, GetReportsState contains a copy of - // mNumChildProcesses which it uses to determine finished-ness. So... + // middle of a request? Well, GetReportsState is initialized with an array + // of child process actors as of when the report started. So... // - // - If a process is created, it won't have received the request for reports, - // and the GetReportsState's mNumChildProcesses won't account for it. So - // the reported data will reflect how things were when the request began. + // - If a process is created after reporting starts, it won't be sent a + // request for reports. So the reported data will reflect how things were + // when the request began. // // - If a process is destroyed before it starts reporting back, the reported // data will reflect how things are when the request ends. @@ -127,7 +131,7 @@ public: // void HandleChildReport(uint32_t aGeneration, const mozilla::dom::MemoryReport& aChildReport); - void EndChildReport(uint32_t aGeneration, bool aSuccess); + void EndProcessReport(uint32_t aGeneration, bool aSuccess); // Functions that (a) implement distinguished amounts, and (b) are outside of // this module. @@ -199,7 +203,6 @@ private: StrongReportersTable* mSavedStrongReporters; WeakReportersTable* mSavedWeakReporters; - uint32_t mNumChildProcesses; uint32_t mNextGeneration; struct GetReportsState @@ -208,8 +211,13 @@ private: bool mAnonymize; bool mMinimize; nsCOMPtr mTimer; - uint32_t mNumChildProcesses; - uint32_t mNumChildProcessesCompleted; + // This is a pointer to an nsTArray because otherwise C++ is + // unhappy unless this header includes ContentParent.h, which not + // everything that includes this header knows how to find. + nsTArray>* mChildrenPending; + uint32_t mNumProcessesRunning; + uint32_t mNumProcessesCompleted; + uint32_t mConcurrencyLimit; nsCOMPtr mHandleReport; nsCOMPtr mHandleReportData; nsCOMPtr mFinishReporting; @@ -217,7 +225,7 @@ private: nsString mDMDDumpIdent; GetReportsState(uint32_t aGeneration, bool aAnonymize, bool aMinimize, - uint32_t aNumChildProcesses, + uint32_t aConcurrencyLimit, nsIHandleReportCallback* aHandleReport, nsISupports* aHandleReportData, nsIFinishReportingCallback* aFinishReporting, @@ -226,8 +234,10 @@ private: : mGeneration(aGeneration) , mAnonymize(aAnonymize) , mMinimize(aMinimize) - , mNumChildProcesses(aNumChildProcesses) - , mNumChildProcessesCompleted(0) + , mChildrenPending(nullptr) + , mNumProcessesRunning(1) // reporting starts with the parent + , mNumProcessesCompleted(0) + , mConcurrencyLimit(aConcurrencyLimit) , mHandleReport(aHandleReport) , mHandleReportData(aHandleReportData) , mFinishReporting(aFinishReporting) @@ -235,6 +245,8 @@ private: , mDMDDumpIdent(aDMDDumpIdent) { } + + ~GetReportsState(); }; // When this is non-null, a request is in flight. Note: We use manual @@ -243,6 +255,8 @@ private: GetReportsState* mGetReportsState; GetReportsState* GetStateForGeneration(uint32_t aGeneration); + static bool StartChildReport(mozilla::dom::ContentParent* aChild, + const GetReportsState* aState); }; #define NS_MEMORY_REPORTER_MANAGER_CID \