Bug 1194555 - Part 6: Run reporters asynchronously. r=njn,jld,ted
authorEric Rahm <erahm@mozilla.com>
Wed, 14 Oct 2015 16:52:59 -0700
changeset 301186 dcee00c2b89dbb1c26720228d28672aa10264126
parent 301185 8db37a722f8326f39042dd2f38d0814fc20ad552
child 301187 da0ad09260848f0e3fe2f2f483710c5bf9a30d68
push id5392
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:08:23 +0000
treeherdermozilla-beta@16ce8562a975 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn, jld, ted
bugs1194555
milestone44.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 1194555 - Part 6: Run reporters asynchronously. r=njn,jld,ted
dom/ipc/ContentChild.cpp
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
toolkit/crashreporter/test/unit/crasher_subprocess_head.js
toolkit/crashreporter/test/unit/crasher_subprocess_tail.js
toolkit/crashreporter/test/unit/test_crash_with_memory_report.js
xpcom/base/nsIMemoryReporter.idl
xpcom/base/nsMemoryReporterManager.cpp
xpcom/base/nsMemoryReporterManager.h
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -895,16 +895,42 @@ private:
     nsRefPtr<MemoryReportRequestChild> mActor;
     const nsCString mProcess;
 };
 NS_IMPL_ISUPPORTS(
   MemoryReportCallback
 , nsIMemoryReporterCallback
 )
 
+class MemoryReportFinishedCallback final : public nsIFinishReportingCallback
+{
+public:
+    NS_DECL_ISUPPORTS
+
+    explicit MemoryReportFinishedCallback(MemoryReportRequestChild* aActor)
+    : mActor(aActor)
+    {
+    }
+
+    NS_IMETHOD Callback(nsISupports* aUnused) override
+    {
+        bool sent = PMemoryReportRequestChild::Send__delete__(mActor);
+        return sent ? NS_OK : NS_ERROR_FAILURE;
+    }
+
+private:
+    ~MemoryReportFinishedCallback() {}
+
+    nsRefPtr<MemoryReportRequestChild> mActor;
+};
+NS_IMPL_ISUPPORTS(
+  MemoryReportFinishedCallback
+, nsIFinishReportingCallback
+)
+
 bool
 ContentChild::RecvPMemoryReportRequestConstructor(
     PMemoryReportRequestChild* aChild,
     const uint32_t& aGeneration,
     const bool& aAnonymize,
     const bool& aMinimizeMemoryUsage,
     const MaybeFileDesc& aDMDFile)
 {
@@ -931,21 +957,22 @@ NS_IMETHODIMP MemoryReportRequestChild::
     nsCString process;
     child->GetProcessName(process);
     child->AppendProcessId(process);
 
     // Run the reporters.  The callback will turn each measurement into a
     // MemoryReport.
     nsRefPtr<MemoryReportCallback> cb =
         new MemoryReportCallback(this, process);
-    mgr->GetReportsForThisProcessExtended(cb, nullptr, mAnonymize,
-                                          FileDescriptorToFILE(mDMDFile, "wb"));
-
-    bool sent = Send__delete__(this);
-    return sent ? NS_OK : NS_ERROR_FAILURE;
+    nsRefPtr<MemoryReportFinishedCallback> finished =
+        new MemoryReportFinishedCallback(this);
+
+    return mgr->GetReportsForThisProcessExtended(cb, nullptr, mAnonymize,
+                                                 FileDescriptorToFILE(mDMDFile, "wb"),
+                                                 finished, nullptr);
 }
 
 bool
 ContentChild::RecvDataStoreNotify(const uint32_t& aAppId,
                                   const nsString& aName,
                                   const nsString& aManifestURL)
 {
   nsRefPtr<DataStoreService> service = DataStoreService::GetOrCreate();
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -547,17 +547,24 @@ End of 5th\n\
       let measureButton = doc.getElementById("measureButton");
       let verbose = doc.getElementById("verbose");
       verbose.checked = aVerbose;
       measureButton.click();
 
       SimpleTest.waitForClipboard(
         function(aActual) {
           mostRecentActual = aActual;
-          return aActual.trim() === aExpected.trim();
+          let rslt = aActual.trim() === aExpected.trim();
+          if (!rslt) {
+            // Try copying again.
+            synthesizeKey("A", {accelKey: true});
+            synthesizeKey("C", {accelKey: true});
+          }
+
+          return rslt;
         },
         function() {
           synthesizeKey("A", {accelKey: true});
           synthesizeKey("C", {accelKey: true});
         },
         aNext,
         function() {
           ok(false, "pasted text doesn't match for " + aFrameId);
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory2.xul
@@ -101,17 +101,24 @@
     }
 
     SimpleTest.executeSoon(function() {
       let mostRecentActual;
       document.getElementById("amFrame").focus();
       SimpleTest.waitForClipboard(
         function(aActual) {
           mostRecentActual = aActual;
-          return aActual.trim() === aExpected.trim();
+          let rslt = aActual.trim() === aExpected.trim();
+          if (!rslt) {
+            // Try copying again.
+            synthesizeKey("A", {accelKey: true});
+            synthesizeKey("C", {accelKey: true});
+          }
+
+          return rslt;
         },
         function() {
           synthesizeKey("A", {accelKey: true});
           synthesizeKey("C", {accelKey: true});
         },
         aNext,
         function() {
           ok(false, "pasted text doesn't match");
--- a/toolkit/crashreporter/test/unit/crasher_subprocess_head.js
+++ b/toolkit/crashreporter/test/unit/crasher_subprocess_head.js
@@ -25,8 +25,9 @@ if (processType == Components.interfaces
 var ios = Components.classes["@mozilla.org/network/io-service;1"]
             .getService(Components.interfaces.nsIIOService);
 var protocolHandler = ios.getProtocolHandler("resource")
                         .QueryInterface(Components.interfaces.nsIResProtocolHandler);
 var curDirURI = ios.newFileURI(cwd);
 protocolHandler.setSubstitution("test", curDirURI);
 Components.utils.import("resource://test/CrashTestUtils.jsm");
 var crashType = CrashTestUtils.CRASH_INVALID_POINTER_DEREF;
+var shouldDelay = false;
--- a/toolkit/crashreporter/test/unit/crasher_subprocess_tail.js
+++ b/toolkit/crashreporter/test/unit/crasher_subprocess_tail.js
@@ -1,2 +1,15 @@
+// Let the event loop process a bit before crashing.
+if (shouldDelay) {
+  let shouldCrashNow = false;
+  let thr = Components.classes["@mozilla.org/thread-manager;1"]
+                          .getService().currentThread;
+  thr.dispatch({ run: () => { shouldCrashNow = true; } },
+               Components.interfaces.nsIThread.DISPATCH_NORMAL);
+
+  while (!shouldCrashNow) {
+    thr.processNextEvent(true);
+  }
+}
+
 // now actually crash
 CrashTestUtils.crash(crashType);
--- a/toolkit/crashreporter/test/unit/test_crash_with_memory_report.js
+++ b/toolkit/crashreporter/test/unit/test_crash_with_memory_report.js
@@ -6,16 +6,19 @@ function run_test()
   }
 
   // This was shamelessly copied and stripped down from do_get_profile() in
   // head.js so that nsICrashReporter::saveMemoryReport can use a profile
   // within the crasher subprocess.
 
   do_crash(
    function() {
+      // Delay crashing so that the memory report has time to complete.
+      shouldDelay = true;
+
       let Cc = Components.classes;
       let Ci = Components.interfaces;
 
       let env = Cc["@mozilla.org/process/environment;1"]
                   .getService(Ci.nsIEnvironment);
       let profd = env.get("XPCSHELL_TEST_PROFILE_DIR");
       let file = Cc["@mozilla.org/file/local;1"]
                    .createInstance(Ci.nsILocalFile);
--- a/xpcom/base/nsIMemoryReporter.idl
+++ b/xpcom/base/nsIMemoryReporter.idl
@@ -200,17 +200,17 @@ interface nsIMemoryReporter : nsISupport
 };
 
 [scriptable, function, uuid(548b3909-c04d-4ca6-8466-b8bee3837457)]
 interface nsIFinishReportingCallback : nsISupports
 {
   void callback(in nsISupports data);
 };
 
-[scriptable, builtinclass, uuid(d473bb53-4b77-48c4-aa0e-921f7d748dae)]
+[scriptable, builtinclass, uuid(61de6dc7-ed11-4104-a577-79941f22f434)]
 interface nsIMemoryReporterManager : nsISupports
 {
   /*
    * Initialize.
    */
   void init();
 
   /*
@@ -294,17 +294,24 @@ interface nsIMemoryReporterManager : nsI
   /*
    * As above, but if DMD is enabled and |DMDFile| is non-null then
    * write a DMD report to that file and close it.
    */
   [noscript] void
     getReportsForThisProcessExtended(in nsIMemoryReporterCallback handleReport,
                                      in nsISupports handleReportData,
                                      in boolean anonymize,
-                                     in FILE DMDFile);
+                                     in FILE DMDFile,
+                                     in nsIFinishReportingCallback finishReporting,
+                                     in nsISupports finishReportingData);
+
+  /*
+   * Called by an asynchronous memory reporter upon completion.
+   */
+  [noscript] void endReport();
 
   /*
    * The memory reporter manager, for the most part, treats reporters
    * registered with it as a black box.  However, there are some
    * "distinguished" amounts (as could be reported by a memory reporter) that
    * the manager provides as attributes, because they are sufficiently
    * interesting that we want external code (e.g. telemetry) to be able to rely
    * on them.
--- a/xpcom/base/nsMemoryReporterManager.cpp
+++ b/xpcom/base/nsMemoryReporterManager.cpp
@@ -1352,16 +1352,17 @@ nsMemoryReporterManager::nsMemoryReporte
   : mMutex("nsMemoryReporterManager::mMutex")
   , mIsRegistrationBlocked(false)
   , mStrongReporters(new StrongReportersTable())
   , mWeakReporters(new WeakReportersTable())
   , mSavedStrongReporters(nullptr)
   , mSavedWeakReporters(nullptr)
   , mNextGeneration(1)
   , mPendingProcessesState(nullptr)
+  , mPendingReportersState(nullptr)
 {
 }
 
 nsMemoryReporterManager::~nsMemoryReporterManager()
 {
   delete mStrongReporters;
   delete mWeakReporters;
   NS_ASSERTION(!mSavedStrongReporters, "failed to restore strong reporters");
@@ -1461,18 +1462,21 @@ nsMemoryReporterManager::StartGettingRep
     rv = nsMemoryInfoDumper::OpenDMDFile(s->mDMDDumpIdent, getpid(),
                                                   &parentDMDFile);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       // Proceed with the memory report as if DMD were disabled.
       parentDMDFile = nullptr;
     }
   }
 #endif
+
+  // This is async.
   GetReportsForThisProcessExtended(s->mHandleReport, s->mHandleReportData,
-                                   s->mAnonymize, parentDMDFile);
+                                   s->mAnonymize, parentDMDFile,
+                                   s->mFinishReporting, s->mFinishReportingData);
 
   nsTArray<ContentParent*> 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.
@@ -1494,63 +1498,117 @@ nsMemoryReporterManager::StartGettingRep
       FinishReporting();
       return rv;
     }
 
     MOZ_ASSERT(!s->mTimer);
     s->mTimer.swap(timer);
   }
 
-  // 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;
 }
 
+void
+nsMemoryReporterManager::DispatchReporter(
+  nsIMemoryReporter* aReporter, bool aIsAsync,
+  nsIHandleReportCallback* aHandleReport,
+  nsISupports* aHandleReportData,
+  bool aAnonymize)
+{
+  MOZ_ASSERT(mPendingReportersState);
+
+  // Grab refs to everything used in the lambda function.
+  nsRefPtr<nsMemoryReporterManager> self = this;
+  nsCOMPtr<nsIMemoryReporter> reporter = aReporter;
+  nsCOMPtr<nsIHandleReportCallback> handleReport = aHandleReport;
+  nsCOMPtr<nsISupports> handleReportData = aHandleReportData;
+
+  nsCOMPtr<nsIRunnable> event = NS_NewRunnableFunction(
+    [self, reporter, aIsAsync, handleReport, handleReportData, aAnonymize] () {
+      reporter->CollectReports(handleReport,
+                               handleReportData,
+                               aAnonymize);
+      if (!aIsAsync) {
+        self->EndReport();
+      }
+    });
+
+  NS_DispatchToMainThread(event);
+  mPendingReportersState->mReportsPending++;
+}
+
 NS_IMETHODIMP
 nsMemoryReporterManager::GetReportsForThisProcessExtended(
   nsIHandleReportCallback* aHandleReport, nsISupports* aHandleReportData,
-  bool aAnonymize, FILE* aDMDFile)
+  bool aAnonymize, FILE* aDMDFile,
+  nsIFinishReportingCallback* aFinishReporting,
+  nsISupports* aFinishReportingData)
 {
   // Memory reporters are not necessarily threadsafe, so this function must
   // be called from the main thread.
   if (!NS_IsMainThread()) {
     MOZ_CRASH();
   }
 
+  if (NS_WARN_IF(mPendingReportersState)) {
+    // Report is already in progress.
+    return NS_ERROR_IN_PROGRESS;
+  }
+
 #ifdef MOZ_DMD
   if (aDMDFile) {
     // Clear DMD's reportedness state before running the memory
     // reporters, to avoid spurious twice-reported warnings.
     dmd::ClearReports();
   }
 #else
   MOZ_ASSERT(!aDMDFile);
 #endif
 
-  nsCOMArray<nsIMemoryReporter> allReporters;
+  mPendingReportersState = new PendingReportersState(
+      aFinishReporting, aFinishReportingData, aDMDFile);
+
   {
     mozilla::MutexAutoLock autoLock(mMutex);
+
     for (auto iter = mStrongReporters->Iter(); !iter.Done(); iter.Next()) {
-      allReporters.AppendElement(iter.Key());
+      DispatchReporter(iter.Key(), iter.Data(),
+                       aHandleReport, aHandleReportData, aAnonymize);
     }
+
     for (auto iter = mWeakReporters->Iter(); !iter.Done(); iter.Next()) {
-      allReporters.AppendElement(iter.Key());
+      nsCOMPtr<nsIMemoryReporter> reporter = iter.Key();
+      DispatchReporter(reporter, iter.Data(),
+                       aHandleReport, aHandleReportData, aAnonymize);
     }
   }
-  for (uint32_t i = 0; i < allReporters.Length(); i++) {
-    allReporters[i]->CollectReports(aHandleReport, aHandleReportData,
-                                    aAnonymize);
-  }
+
+  return NS_OK;
+}
 
+NS_IMETHODIMP
+nsMemoryReporterManager::EndReport()
+{
+  if (--mPendingReportersState->mReportsPending == 0) {
 #ifdef MOZ_DMD
-  if (aDMDFile) {
-    return nsMemoryInfoDumper::DumpDMDToFile(aDMDFile);
+    if (mPendingReportersState->mDMDFile) {
+      nsMemoryInfoDumper::DumpDMDToFile(mPendingReportersState->mDMDFile);
+    }
+#endif
+    if (mPendingProcessesState) {
+      // This is the parent process.
+      EndProcessReport(mPendingProcessesState->mGeneration, true);
+    } else {
+      mPendingReportersState->mFinishReporting->Callback(
+          mPendingReportersState->mFinishReportingData);
+    }
+
+    delete mPendingReportersState;
+    mPendingReportersState = nullptr;
   }
-#endif
 
   return NS_OK;
 }
 
 nsMemoryReporterManager::PendingProcessesState*
 nsMemoryReporterManager::GetStateForGeneration(uint32_t aGeneration)
 {
   // Memory reporting only happens on the main thread.
--- a/xpcom/base/nsMemoryReporterManager.h
+++ b/xpcom/base/nsMemoryReporterManager.h
@@ -183,16 +183,21 @@ public:
   SizeOfTabFns mSizeOfTabFns;
 
 private:
   nsresult RegisterReporterHelper(nsIMemoryReporter* aReporter,
                                   bool aForce, bool aStrongRef, bool aIsAsync);
   nsresult StartGettingReports();
   nsresult FinishReporting();
 
+  void DispatchReporter(nsIMemoryReporter* aReporter, bool aIsAsync,
+                        nsIHandleReportCallback* aHandleReport,
+                        nsISupports* aHandleReportData,
+                        bool aAnonymize);
+
   static void TimeoutCallback(nsITimer* aTimer, void* aData);
   // Note: this timeout needs to be long enough to allow for the
   // possibility of DMD reports and/or running on a low-end phone.
   static const uint32_t kTimeoutLengthMS = 50000;
 
   mozilla::Mutex mMutex;
   bool mIsRegistrationBlocked;
 
@@ -200,16 +205,19 @@ private:
   WeakReportersTable* mWeakReporters;
 
   // These two are only used for testing purposes.
   StrongReportersTable* mSavedStrongReporters;
   WeakReportersTable* mSavedWeakReporters;
 
   uint32_t mNextGeneration;
 
+  // Used to keep track of state of which processes are currently running and
+  // waiting to run memory reports. Holds references to parameters needed when
+  // requesting a memory report and finishing reporting.
   struct PendingProcessesState
   {
     uint32_t                             mGeneration;
     bool                                 mAnonymize;
     bool                                 mMinimize;
     nsCOMPtr<nsITimer>                   mTimer;
     nsTArray<nsRefPtr<mozilla::dom::ContentParent>> mChildrenPending;
     uint32_t                             mNumProcessesRunning;
@@ -225,21 +233,50 @@ private:
                           uint32_t aConcurrencyLimit,
                           nsIHandleReportCallback* aHandleReport,
                           nsISupports* aHandleReportData,
                           nsIFinishReportingCallback* aFinishReporting,
                           nsISupports* aFinishReportingData,
                           const nsAString& aDMDDumpIdent);
   };
 
+  // Used to keep track of the state of the asynchronously run memory
+  // reporters. The callback and file handle used when all memory reporters
+  // have finished are also stored here.
+  struct PendingReportersState
+  {
+    // Number of memory reporters currently running.
+    uint32_t mReportsPending;
+
+    // Callback for when all memory reporters have completed.
+    nsCOMPtr<nsIFinishReportingCallback> mFinishReporting;
+    nsCOMPtr<nsISupports> mFinishReportingData;
+
+    // File handle to write a DMD report to if requested.
+    FILE* mDMDFile;
+
+    PendingReportersState(nsIFinishReportingCallback* aFinishReporting,
+                        nsISupports* aFinishReportingData,
+                        FILE* aDMDFile)
+      : mReportsPending(0)
+      , mFinishReporting(aFinishReporting)
+      , mFinishReportingData(aFinishReportingData)
+      , mDMDFile(aDMDFile)
+    {
+    }
+  };
+
   // When this is non-null, a request is in flight.  Note: We use manual
   // new/delete for this because its lifetime doesn't match block scope or
   // anything like that.
   PendingProcessesState* mPendingProcessesState;
 
+  // This is reinitialized each time a call to GetReports is initiated.
+  PendingReportersState* mPendingReportersState;
+
   PendingProcessesState* GetStateForGeneration(uint32_t aGeneration);
   static bool StartChildReport(mozilla::dom::ContentParent* aChild,
                                const PendingProcessesState* aState);
 };
 
 #define NS_MEMORY_REPORTER_MANAGER_CID \
 { 0xfb97e4f5, 0x32dd, 0x497a, \
 { 0xba, 0xa2, 0x7d, 0x1e, 0x55, 0x7, 0x99, 0x10 } }