Bug 1350967 (part 2) - Remove profiler_get_profile_jsobject_async() and profiler_save_profile_to_file_async(). r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 29 Mar 2017 15:48:13 +1100
changeset 400225 a1e5043844f295aa8f9a70517b7b76e34fecc634
parent 400224 4483302f97a757145dca7dd5720a397d4f9b9246
child 400226 6bf903b1e7ccdd88a688b988756e78ddd7014741
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1350967
milestone55.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 1350967 (part 2) - Remove profiler_get_profile_jsobject_async() and profiler_save_profile_to_file_async(). r=mstange. The state management is better done within nsProfiler::GetProfileDataAsync() and nsProfiler::DumpProfileToFileAsync(). (The latter function is new in this patch.) This fixes a deadlock. Other notes: - The patch moves ProfileGatherer from ProfilerState to nsProfiler. This is nice because the former is shared between threads but the latter is main thread only. (This is how the deadlock is avoided.) - ProfilerStateMutex and PSLockRef are no longer required in platform.h. Those types and variables are now only used in platform.cpp and platform-*.cpp. - ProfilerGatherer now calls profiler_get_profile() instead of ToJSON(). Which means that ToJSON() now has a single caller, so the patch inlines it at the callsite and removes it. - profiler_save_profile_to_file_async() dispatched a Runnable to the main thread. But this wasn't necessary, because it always ran on the main thread itself. So the new function nsProfiler::DumpProfileToFileAsync() doesn't do that. - profiler_will_gather_OOP_profile(), profiler_gathered_OOP_profile(), and profiler_OOP_exit_profile() are all moved into nsProfiler as well. This removes the need for the horrible fake lock in profiler_will_gather_OOP_profile(), hooray!
tools/profiler/core/platform.cpp
tools/profiler/core/platform.h
tools/profiler/gecko/CrossProcessProfilerController.cpp
tools/profiler/gecko/ProfileGatherer.cpp
tools/profiler/gecko/ProfileGatherer.h
tools/profiler/gecko/nsIProfiler.idl
tools/profiler/gecko/nsProfiler.cpp
tools/profiler/gecko/nsProfiler.h
tools/profiler/public/GeckoProfiler.h
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -33,19 +33,19 @@
 #include "nsIXULRuntime.h"
 #include "nsDirectoryServiceUtils.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsMemoryReporterManager.h"
 #include "nsXULAppAPI.h"
 #include "nsProfilerStartParams.h"
 #include "mozilla/Services.h"
 #include "nsThreadUtils.h"
-#include "ProfileGatherer.h"
 #include "ProfilerMarkers.h"
 #include "shared-libraries.h"
+#include "prtime.h"
 
 #ifdef MOZ_TASK_TRACER
 #include "GeckoTaskTracer.h"
 #endif
 
 #if defined(PROFILE_JAVA)
 # include "FennecJNINatives.h"
 # include "FennecJNIWrappers.h"
@@ -124,17 +124,18 @@ MOZ_THREAD_LOCAL(PseudoStack *) tlsPseud
 //
 // Other from the lock protection, this class is essentially a thin wrapper and
 // contains very little "smarts" itself.
 //
 class ProfilerState
 {
 public:
   // Shorter names for local use.
-  typedef ProfilerStateMutex Mutex;
+  class Mutex : public mozilla::StaticMutex {};
+
   typedef mozilla::BaseAutoLock<Mutex> AutoLock;
 
   // Only functions that take a LockRef arg can modify this class's fields.
   typedef const AutoLock& LockRef;
 
   typedef std::vector<ThreadInfo*> ThreadVector;
 
   ProfilerState()
@@ -148,17 +149,16 @@ public:
     , mFeatureLeaf(false)
     , mFeatureMemory(false)
     , mFeaturePrivacy(false)
     , mFeatureRestyle(false)
     , mFeatureStackWalk(false)
     , mFeatureTaskTracer(false)
     , mFeatureThreads(false)
     , mBuffer(nullptr)
-    , mGatherer(nullptr)
     , mIsPaused(false)
 #if defined(GP_OS_linux) || defined(GP_OS_android)
     , mWasPaused(false)
 #endif
     , mSamplerThread(nullptr)
 #ifdef USE_LUL_STACKWALK
     , mLUL(nullptr)
 #endif
@@ -191,18 +191,16 @@ public:
   GET_AND_SET(bool, FeaturePrivacy)
   GET_AND_SET(bool, FeatureRestyle)
   GET_AND_SET(bool, FeatureStackWalk)
   GET_AND_SET(bool, FeatureTaskTracer)
   GET_AND_SET(bool, FeatureThreads)
 
   GET_AND_SET(ProfileBuffer*, Buffer)
 
-  GET_AND_SET(ProfileGatherer*, Gatherer)
-
   ThreadVector& Threads(LockRef) { return mThreads; }
 
   static bool IsActive(LockRef) { return sActivityGeneration > 0; }
   static uint32_t ActivityGeneration(LockRef) { return sActivityGeneration; }
   static void SetInactive(LockRef) { sActivityGeneration = 0; }
   static void SetActive(LockRef)
   {
     sActivityGeneration = sNextActivityGeneration;
@@ -264,20 +262,16 @@ private:
   bool mFeatureStackWalk;
   bool mFeatureTaskTracer;
   bool mFeatureThreads;
 
   // The buffer into which all samples are recorded. Always used in conjunction
   // with mThreads. Null when the profiler is inactive.
   ProfileBuffer* mBuffer;
 
-  // A helper class that is used when saving profiles. Null when the profiler
-  // is inactive.
-  RefPtr<mozilla::ProfileGatherer> mGatherer;
-
   // All the registered threads.
   ThreadVector mThreads;
 
   // Is the profiler active? The obvious way to track this is with a bool,
   // sIsActive, but then we could have the following scenario.
   //
   // - profiler_stop() locks gPSMutex, zeroes sIsActive, unlocks gPSMutex,
   //   deletes the SamplerThread (which does a join).
@@ -1441,29 +1435,16 @@ StreamJSON(PS::LockRef aLock, Spliceable
 
       gPS->SetIsPaused(aLock, false);
     }
     aWriter.EndArray();
   }
   aWriter.End();
 }
 
-UniquePtr<char[]>
-ToJSON(PS::LockRef aLock, double aSinceTime)
-{
-  LOG("ToJSON");
-
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS && gPS->IsActive(aLock));
-
-  SpliceableChunkedJSONWriter b;
-  StreamJSON(aLock, b, aSinceTime);
-  return b.WriteFunc()->CopyData();
-}
-
 // END saving/streaming code
 ////////////////////////////////////////////////////////////////////////
 
 ProfilerMarker::ProfilerMarker(const char* aMarkerName,
                                ProfilerMarkerPayload* aPayload,
                                double aTime)
   : mMarkerName(strdup(aMarkerName))
   , mPayload(aPayload)
@@ -1791,17 +1772,16 @@ GeckoProfilerReporter::CollectReports(ns
           gPS->Buffer(lock)->SizeOfIncludingThis(GeckoProfilerMallocSizeOf);
       }
 
       // Measurement of the following things may be added later if DMD finds it
       // is worthwhile:
       // - gPS->mFeatures
       // - gPS->mThreadNameFilters
       // - gPS->mThreads itself (its elements' children are measured above)
-      // - gPS->mGatherer
       // - gPS->mInterposeObserver
 
 #if defined(USE_LUL_STACKWALK)
       lul::LUL* lul = gPS->LUL(lock);
       lulSize = lul ? lul->SizeOfIncludingThis(GeckoProfilerMallocSizeOf) : 0;
 #endif
     }
   }
@@ -2139,62 +2119,19 @@ profiler_get_profile(double aSinceTime)
   MOZ_RELEASE_ASSERT(gPS);
 
   PS::AutoLock lock(gPSMutex);
 
   if (!gPS->IsActive(lock)) {
     return nullptr;
   }
 
-  return ToJSON(lock, aSinceTime);
-}
-
-void
-profiler_get_profile_jsobject_async(double aSinceTime,
-                                    mozilla::dom::Promise* aPromise)
-{
-  LOG("profiler_get_profile_jsobject_async");
-
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS);
-
-  PS::AutoLock lock(gPSMutex);
-
-  if (!gPS->IsActive(lock)) {
-    LOG("END   profiler_get_profile_jsobject_async: inactive");
-    return;
-  }
-
-  gPS->Gatherer(lock)->Start(lock, aSinceTime, aPromise);
-}
-
-void
-profiler_save_profile_to_file_async(double aSinceTime, const char* aFileName)
-{
-  LOG("BEGIN profiler_save_profile_to_file_async");
-
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS);
-
-  nsCString filename(aFileName);
-  NS_DispatchToMainThread(NS_NewRunnableFunction([=] () {
-
-    LOG("profiler_save_profile_to_file_async callback");
-
-    PS::AutoLock lock(gPSMutex);
-
-    // It's conceivable that profiler_stop() or profiler_shutdown() was called
-    // between the dispatch and running of this runnable, so check for those.
-    if (!gPS || !gPS->IsActive(lock)) {
-      LOG("END   profiler_save_profile_to_file_async callback: inactive");
-      return;
-    }
-
-    gPS->Gatherer(lock)->Start(lock, aSinceTime, filename);
-  }));
+  SpliceableChunkedJSONWriter b;
+  StreamJSON(lock, b, aSinceTime);
+  return b.WriteFunc()->CopyData();
 }
 
 void
 profiler_get_start_params(int* aEntries, double* aInterval,
                           mozilla::Vector<const char*>* aFeatures,
                           mozilla::Vector<const char*>* aFilters)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
@@ -2218,71 +2155,16 @@ profiler_get_start_params(int* aEntries,
 
   const Vector<std::string>& threadNameFilters = gPS->ThreadNameFilters(lock);
   MOZ_ALWAYS_TRUE(aFilters->resize(threadNameFilters.length()));
   for (uint32_t i = 0; i < threadNameFilters.length(); ++i) {
     (*aFilters)[i] = threadNameFilters[i].c_str();
   }
 }
 
-void
-profiler_will_gather_OOP_profile()
-{
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS);
-
-  // This function is called once per subprocess in response to the observation
-  // of a "profile-subprocess-gather" notification. That notification
-  // originates from ProfileGatherer::Start2(). The observers receive it and
-  // immediately call this function, all while Start2() holds gPSMutex locked.
-  // This is non-trivial, so we assert that gPSMutex is locked as expected...
-  gPSMutex.AssertCurrentThreadOwns();
-
-  // ...therefore we don't need to lock gPSMutex. But we need a PS::AutoLock to
-  // access gPS, so we make a fake one. This is gross but it's hard to get the
-  // "profile-subprocess-gather" observers to call back here any other way
-  // without exposing ProfileGatherer, which causes other difficulties.
-  static PS::Mutex sFakeMutex;
-  PS::AutoLock fakeLock(sFakeMutex);
-
-  MOZ_RELEASE_ASSERT(gPS->IsActive(fakeLock));
-
-  gPS->Gatherer(fakeLock)->WillGatherOOPProfile();
-}
-
-void
-profiler_gathered_OOP_profile()
-{
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS);
-
-  PS::AutoLock lock(gPSMutex);
-
-  if (!gPS->IsActive(lock)) {
-    return;
-  }
-
-  gPS->Gatherer(lock)->GatheredOOPProfile(lock);
-}
-
-void
-profiler_OOP_exit_profile(const nsCString& aProfile)
-{
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  MOZ_RELEASE_ASSERT(gPS);
-
-  PS::AutoLock lock(gPSMutex);
-
-  if (!gPS->IsActive(lock)) {
-    return;
-  }
-
-  gPS->Gatherer(lock)->OOPExitProfile(aProfile);
-}
-
 static void
 locked_profiler_save_profile_to_file(PS::LockRef aLock, const char* aFilename)
 {
   LOG("locked_profiler_save_profile_to_file(%s)", aFilename);
 
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   MOZ_RELEASE_ASSERT(gPS && gPS->IsActive(aLock));
 
@@ -2459,18 +2341,16 @@ locked_profiler_start(PS::LockRef aLock,
   // to filter by a list of threads but forget to explicitly request.
   // Must precede the MaybeSetProfile() call below.
   gPS->SetFeatureThreads(aLock, HAS_FEATURE("threads") || aFilterCount > 0);
 
 #undef HAS_FEATURE
 
   gPS->SetBuffer(aLock, new ProfileBuffer(entries));
 
-  gPS->SetGatherer(aLock, new mozilla::ProfileGatherer());
-
   // Set up profiling for each registered thread, if appropriate.
   const PS::ThreadVector& threads = gPS->Threads(aLock);
   for (uint32_t i = 0; i < threads.size(); i++) {
     ThreadInfo* info = threads.at(i);
 
     MaybeSetProfile(aLock, info);
 
     if (info->HasProfile() && !info->IsPendingDelete()) {
@@ -2613,20 +2493,16 @@ locked_profiler_stop(PS::LockRef aLock)
   if (gPS->FeatureJS(aLock)) {
     // We just called stopJSSampling() on all relevant threads. We can also
     // manually poll the current thread so it stops profiling immediately.
     if (PseudoStack* stack = tlsPseudoStack.get()) {
       stack->pollJSSampling();
     }
   }
 
-  // Cancel any in-flight async profile gathering requests.
-  gPS->Gatherer(aLock)->Cancel();
-  gPS->SetGatherer(aLock, nullptr);
-
   delete gPS->Buffer(aLock);
   gPS->SetBuffer(aLock, nullptr);
 
   gPS->SetFeatureDisplayListDump(aLock, false);
   gPS->SetFeatureGPU(aLock, false);
   gPS->SetFeatureJava(aLock, false);
   gPS->SetFeatureJS(aLock, false);
   gPS->SetFeatureLayersDump(aLock, false);
--- a/tools/profiler/core/platform.h
+++ b/tools/profiler/core/platform.h
@@ -107,48 +107,28 @@ public:
 #else
   typedef ::pid_t tid_t;
 #endif
 
   static tid_t GetCurrentId();
 };
 
 // ----------------------------------------------------------------------------
-// ProfilerState auxiliaries
-
-// There is a single instance of ProfilerState that holds the profiler's global
-// state. Both the class and the instance are defined in platform.cpp.
-// ProfilerStateMutex is the type of the mutex that guards all accesses to
-// ProfilerState. We use a distinct type for it to make it hard to mix up with
-// any other StaticMutex.
-class ProfilerStateMutex : public mozilla::StaticMutex {};
-
-// Values of this type are passed around as a kind of token indicating which
-// functions are called while the global ProfilerStateMutex is held, i.e. while
-// the ProfilerState can be modified. (All of ProfilerState's getters and
-// setters require this token.) Some such functions are outside platform.cpp,
-// so this type must be declared here.
-typedef const mozilla::BaseAutoLock<ProfilerStateMutex>& PSLockRef;
-
-// ----------------------------------------------------------------------------
 // Miscellaneous
 
 class PlatformData;
 
 // We can't new/delete the type safely without defining it
 // (-Wdelete-incomplete).  Use these to hide the details from clients.
 struct PlatformDataDestructor {
   void operator()(PlatformData*);
 };
 
 typedef mozilla::UniquePtr<PlatformData, PlatformDataDestructor>
   UniquePlatformData;
 UniquePlatformData AllocPlatformData(int aThreadId);
 
-mozilla::UniquePtr<char[]>
-ToJSON(PSLockRef aLock, double aSinceTime);
-
 namespace mozilla {
 class JSONWriter;
 }
 void AppendSharedLibraries(mozilla::JSONWriter& aWriter);
 
 #endif /* ndef TOOLS_PLATFORM_H_ */
--- a/tools/profiler/gecko/CrossProcessProfilerController.cpp
+++ b/tools/profiler/gecko/CrossProcessProfilerController.cpp
@@ -5,16 +5,17 @@
 #include "CrossProcessProfilerController.h"
 
 #include "mozilla/Move.h"
 #include "mozilla/ProfilerTypes.h"
 #include "nsIProfiler.h"
 #include "nsIProfileSaveEvent.h"
 #include "nsISupports.h"
 #include "nsIObserver.h"
+#include "nsProfiler.h"
 #include "ProfilerControllingProcess.h"
 
 namespace mozilla {
 
 static const char* sObserverTopics[] = {
   "profiler-started",
   "profiler-stopped",
   "profiler-paused",
@@ -80,17 +81,17 @@ CrossProcessProfilerController::CrossPro
       obs->AddObserver(mObserver, sObserverTopics[i], false);
     }
   }
 }
 
 CrossProcessProfilerController::~CrossProcessProfilerController()
 {
   if (!mProfile.IsEmpty()) {
-    profiler_OOP_exit_profile(mProfile);
+    nsProfiler::GetOrCreate()->OOPExitProfile(mProfile);
   }
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->RemoveObserver(mObserver, sObserverTopics[i]);
     }
@@ -119,17 +120,17 @@ void
 CrossProcessProfilerController::Observe(nsISupports* aSubject,
                                         const char* aTopic)
 {
   if (!strcmp(aTopic, "profiler-subprocess-gather")) {
     // profiler-subprocess-gather is the request to capture the profile. We
     // need to tell the other process that we're interested in its profile,
     // and we tell the gatherer that we've forwarded the request, so that it
     // can keep track of the number of pending profiles.
-    profiler_will_gather_OOP_profile();
+    nsProfiler::GetOrCreate()->WillGatherOOPProfile();
     mProcess->SendGatherProfile();
   }
   else if (!strcmp(aTopic, "profiler-subprocess")) {
     // profiler-subprocess is sent once the gatherer knows that all other
     // processes have replied with their profiles. It's sent during the final
     // assembly of the parent process profile, and this is where we pass the
     // subprocess profile along.
     nsCOMPtr<nsIProfileSaveEvent> pse = do_QueryInterface(aSubject);
@@ -164,12 +165,12 @@ void
 CrossProcessProfilerController::RecvProfile(const nsCString& aProfile)
 {
   // Store the profile on this object.
   mProfile = aProfile;
   // Tell the gatherer that we've received the profile from this process, but
   // don't actually give it the profile. It will request the profile once all
   // processes have replied, through the "profiler-subprocess" observer
   // notification.
-  profiler_gathered_OOP_profile();
+  nsProfiler::GetOrCreate()->GatheredOOPProfile();
 }
 
 } // namespace mozilla
--- a/tools/profiler/gecko/ProfileGatherer.cpp
+++ b/tools/profiler/gecko/ProfileGatherer.cpp
@@ -6,17 +6,16 @@
 
 #include "ProfileGatherer.h"
 
 #include "mozilla/Services.h"
 #include "nsIObserverService.h"
 #include "nsIProfileSaveEvent.h"
 #include "nsLocalFile.h"
 #include "nsIFileStreams.h"
-#include "platform.h"
 
 using mozilla::dom::AutoJSAPI;
 using mozilla::dom::Promise;
 
 namespace mozilla {
 
 /**
  * When a subprocess exits before we've gathered profiles, we'll
@@ -25,25 +24,29 @@ namespace mozilla {
  * circular, so as soon as we receive another exit profile, we'll
  * bump the oldest one out of the buffer.
  */
 static const uint32_t MAX_SUBPROCESS_EXIT_PROFILES = 5;
 
 NS_IMPL_ISUPPORTS(ProfileGatherer, nsIObserver)
 
 ProfileGatherer::ProfileGatherer()
-  : mIsCancelled(false)
-  , mSinceTime(0)
+  : mSinceTime(0)
   , mPendingProfiles(0)
   , mGathering(false)
 {
 }
 
+ProfileGatherer::~ProfileGatherer()
+{
+  Cancel();
+}
+
 void
-ProfileGatherer::GatheredOOPProfile(PSLockRef aLock)
+ProfileGatherer::GatheredOOPProfile()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   if (!mGathering) {
     // If we're not actively gathering, then we don't actually
     // care that we gathered a profile here. This can happen for
     // processes that exit while profiling.
     return;
@@ -55,106 +58,113 @@ ProfileGatherer::GatheredOOPProfile(PSLo
     return;
   }
 
   mPendingProfiles--;
 
   if (mPendingProfiles == 0) {
     // We've got all of the async profiles now. Let's
     // finish off the profile and resolve the Promise.
-    Finish(aLock);
+    Finish();
   }
 }
 
 void
 ProfileGatherer::WillGatherOOPProfile()
 {
   mPendingProfiles++;
 }
 
 void
-ProfileGatherer::Start(PSLockRef aLock, double aSinceTime, Promise* aPromise)
+ProfileGatherer::Start(double aSinceTime, Promise* aPromise)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   if (mGathering) {
     // If we're already gathering, reject the promise - this isn't going
     // to end well.
     if (aPromise) {
       aPromise->MaybeReject(NS_ERROR_NOT_AVAILABLE);
     }
     return;
   }
 
   mPromise = aPromise;
 
-  Start2(aLock, aSinceTime);
+  Start2(aSinceTime);
 }
 
 void
-ProfileGatherer::Start(PSLockRef aLock, double aSinceTime,
-                       const nsACString& aFileName)
+ProfileGatherer::Start(double aSinceTime, const nsACString& aFileName)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   nsCOMPtr<nsIFile> file = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID);
   nsresult rv = file->InitWithNativePath(aFileName);
   if (NS_FAILED(rv)) {
     MOZ_CRASH();
   }
 
   if (mGathering) {
     return;
   }
 
   mFile = file;
 
-  Start2(aLock, aSinceTime);
+  Start2(aSinceTime);
 }
 
 // This is the common tail shared by both Start() methods.
 void
-ProfileGatherer::Start2(PSLockRef aLock, double aSinceTime)
+ProfileGatherer::Start2(double aSinceTime)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   mSinceTime = aSinceTime;
   mGathering = true;
   mPendingProfiles = 0;
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
     DebugOnly<nsresult> rv =
       os->AddObserver(this, "profiler-subprocess", false);
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "AddObserver failed");
 
-    // This notification triggers calls back to
-    // profiler_will_gather_OOP_profile(). See the comment in that function for
-    // the gory details of the connection between this function and that one.
     rv = os->NotifyObservers(this, "profiler-subprocess-gather", nullptr);
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "NotifyObservers failed");
   }
 
   if (!mPendingProfiles) {
-    Finish(aLock);
+    Finish();
   }
 }
 
 void
-ProfileGatherer::Finish(PSLockRef aLock)
+ProfileGatherer::Finish()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
-  if (mIsCancelled) {
-    // We somehow got called after we were cancelled! This shouldn't
-    // be possible, but doing a belt-and-suspenders check to be sure.
+  // It's unlikely but possible that profiler_stop() could be called while the
+  // profile gathering is in flight, but not via nsProfiler::StopProfiler().
+  // This check will detect that case.
+  //
+  // XXX: However, it won't detect the case where profiler_stop() *and*
+  // profiler_start() have both been called. (If that does happen, we'll end up
+  // with a franken-profile that includes a mix of data from the old and new
+  // profile activations.) We could include the activity generation to detect
+  // that, but it's not worth it for what should be an extremely unlikely case.
+  // It would be better if this class was rearranged so that
+  // profiler_get_profile() was called for the parent process in Start2()
+  // instead of in Finish(). Then we wouldn't have to worry about cancelling.
+  if (!profiler_is_active()) {
+    Cancel();
     return;
   }
 
-  UniquePtr<char[]> buf = ToJSON(aLock, mSinceTime);
+  UniquePtr<char[]> buf = profiler_get_profile(mSinceTime);
 
   if (mFile) {
     nsCOMPtr<nsIFileOutputStream> of =
       do_CreateInstance("@mozilla.org/network/file-output-stream;1");
     of->Init(mFile, -1, -1, 0);
     uint32_t sz;
     of->Write(buf.get(), strlen(buf.get()), &sz);
     of->Close();
@@ -216,22 +226,20 @@ ProfileGatherer::Cancel()
 {
   // We're about to stop profiling. If we have a Promise in flight, we should
   // reject it.
   if (mPromise) {
     mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
   }
   mPromise = nullptr;
   mFile = nullptr;
-
-  mIsCancelled = true;
 }
 
 void
-ProfileGatherer::OOPExitProfile(const nsCString& aProfile)
+ProfileGatherer::OOPExitProfile(const nsACString& aProfile)
 {
   if (mExitProfiles.Length() >= MAX_SUBPROCESS_EXIT_PROFILES) {
     mExitProfiles.RemoveElementAt(0);
   }
   mExitProfiles.AppendElement(aProfile);
 }
 
 NS_IMETHODIMP
--- a/tools/profiler/gecko/ProfileGatherer.h
+++ b/tools/profiler/gecko/ProfileGatherer.h
@@ -2,45 +2,43 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef MOZ_PROFILE_GATHERER_H
 #define MOZ_PROFILE_GATHERER_H
 
 #include "mozilla/dom/Promise.h"
 #include "nsIFile.h"
-#include "platform.h"
 
 namespace mozilla {
 
+// This class holds the state for an async profile-gathering request.
 class ProfileGatherer final : public nsIObserver
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   explicit ProfileGatherer();
   void WillGatherOOPProfile();
-  void GatheredOOPProfile(PSLockRef aLock);
-  void Start(PSLockRef aLock, double aSinceTime,
-             mozilla::dom::Promise* aPromise);
-  void Start(PSLockRef aLock, double aSinceTime, const nsACString& aFileName);
+  void GatheredOOPProfile();
+  void Start(double aSinceTime, mozilla::dom::Promise* aPromise);
+  void Start(double aSinceTime, const nsACString& aFileName);
   void Cancel();
-  void OOPExitProfile(const nsCString& aProfile);
+  void OOPExitProfile(const nsACString& aProfile);
 
 private:
-  ~ProfileGatherer() {};
-  void Finish(PSLockRef aLock);
+  ~ProfileGatherer();
+  void Finish();
   void Reset();
-  void Start2(PSLockRef aLock, double aSinceTime);
+  void Start2(double aSinceTime);
 
   nsTArray<nsCString> mExitProfiles;
   RefPtr<mozilla::dom::Promise> mPromise;
   nsCOMPtr<nsIFile> mFile;
-  bool mIsCancelled;
   double mSinceTime;
   uint32_t mPendingProfiles;
   bool mGathering;
 };
 
 } // namespace mozilla
 
 #endif
--- a/tools/profiler/gecko/nsIProfiler.idl
+++ b/tools/profiler/gecko/nsIProfiler.idl
@@ -52,16 +52,19 @@ interface nsIProfiler : nsISupports
    * samples taken at >= aSinceTime.
    */
   [implicit_jscontext]
   jsval getProfileData([optional] in double aSinceTime);
 
   [implicit_jscontext]
   nsISupports getProfileDataAsync([optional] in double aSinceTime);
 
+  void dumpProfileToFileAsync(in ACString aFilename,
+                              [optional] in double aSinceTime);
+
   boolean IsActive();
   void GetFeatures(out uint32_t aCount, [retval, array, size_is(aCount)] out string aFeatures);
 
   /**
    * The starting parameters that were sent to the profiler for sampling.
    * If the profiler is not currently sampling, this will return null.
    */
   readonly attribute nsIProfilerStartParams startParams;
--- a/tools/profiler/gecko/nsProfiler.cpp
+++ b/tools/profiler/gecko/nsProfiler.cpp
@@ -1,9 +1,10 @@
 /* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <string>
 #include <sstream>
 #include "GeckoProfiler.h"
 #include "nsProfiler.h"
@@ -15,16 +16,17 @@
 #include "nsIInterfaceRequestor.h"
 #include "nsILoadContext.h"
 #include "nsIWebNavigation.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "shared-libraries.h"
 #include "js/Value.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/Promise.h"
+#include "ProfileGatherer.h"
 
 using mozilla::ErrorResult;
 using mozilla::dom::Promise;
 using std::string;
 
 NS_IMPL_ISUPPORTS(nsProfiler, nsIProfiler)
 
 nsProfiler::nsProfiler()
@@ -87,23 +89,31 @@ nsProfiler::StartProfiler(uint32_t aEntr
 {
   if (mLockedForPrivateBrowsing) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   profiler_start(aEntries, aInterval,
                  aFeatures, aFeatureCount,
                  aThreadNameFilters, aFilterCount);
+
+  // Do this after profiler_start().
+  mGatherer = new ProfileGatherer();
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProfiler::StopProfiler()
 {
+  // Do this before profiler_stop().
+  mGatherer = nullptr;
+
   profiler_stop();
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProfiler::IsPaused(bool *aIsPaused)
 {
   *aIsPaused = profiler_is_paused();
   return NS_OK;
@@ -206,39 +216,59 @@ nsProfiler::GetProfileData(double aSince
 }
 
 NS_IMETHODIMP
 nsProfiler::GetProfileDataAsync(double aSinceTime, JSContext* aCx,
                                 nsISupports** aPromise)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+  if (!mGatherer) {
+    return NS_ERROR_FAILURE;
+  }
+
   if (NS_WARN_IF(!aCx)) {
     return NS_ERROR_FAILURE;
   }
 
   nsIGlobalObject* go = xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx));
 
   if (NS_WARN_IF(!go)) {
     return NS_ERROR_FAILURE;
   }
 
   ErrorResult result;
   RefPtr<Promise> promise = Promise::Create(go, result);
   if (NS_WARN_IF(result.Failed())) {
     return result.StealNSResult();
   }
 
-  profiler_get_profile_jsobject_async(aSinceTime, promise);
+  mGatherer->Start(aSinceTime, promise);
 
   promise.forget(aPromise);
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsProfiler::DumpProfileToFileAsync(const nsACString& aFilename,
+                                   double aSinceTime)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (!mGatherer) {
+    return NS_ERROR_FAILURE;
+  }
+
+  mGatherer->Start(aSinceTime, aFilename);
+
+  return NS_OK;
+}
+
+
+NS_IMETHODIMP
 nsProfiler::GetElapsedTime(double* aElapsedTime)
 {
   *aElapsedTime = profiler_time();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProfiler::IsActive(bool *aIsActive)
@@ -313,8 +343,44 @@ nsProfiler::GetBufferInfo(uint32_t* aCur
 {
   MOZ_ASSERT(aCurrentPosition);
   MOZ_ASSERT(aTotalSize);
   MOZ_ASSERT(aGeneration);
   profiler_get_buffer_info(aCurrentPosition, aTotalSize, aGeneration);
   return NS_OK;
 }
 
+void
+nsProfiler::WillGatherOOPProfile()
+{
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
+  if (!mGatherer) {
+    return;
+  }
+
+  mGatherer->WillGatherOOPProfile();
+}
+
+void
+nsProfiler::GatheredOOPProfile()
+{
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
+  if (!mGatherer) {
+    return;
+  }
+
+  mGatherer->GatheredOOPProfile();
+}
+
+void
+nsProfiler::OOPExitProfile(const nsACString& aProfile)
+{
+  MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
+  if (!mGatherer) {
+    return;
+  }
+
+  mGatherer->OOPExitProfile(aProfile);
+}
+
--- a/tools/profiler/gecko/nsProfiler.h
+++ b/tools/profiler/gecko/nsProfiler.h
@@ -5,25 +5,43 @@
 
 #ifndef _NSPROFILER_H_
 #define _NSPROFILER_H_
 
 #include "nsIProfiler.h"
 #include "nsIObserver.h"
 #include "mozilla/Attributes.h"
 
+namespace mozilla {
+class ProfileGatherer;
+}
+
 class nsProfiler final : public nsIProfiler, public nsIObserver
 {
 public:
     nsProfiler();
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIOBSERVER
     NS_DECL_NSIPROFILER
 
     nsresult Init();
+
+    static nsProfiler* GetOrCreate()
+    {
+	nsCOMPtr<nsIProfiler> iprofiler =
+	    do_GetService("@mozilla.org/tools/profiler;1");
+	return static_cast<nsProfiler*>(iprofiler.get());
+    }
+
+    void WillGatherOOPProfile();
+    void GatheredOOPProfile();
+    void OOPExitProfile(const nsACString& aProfile);
+
 private:
     ~nsProfiler();
+
+    RefPtr<ProfileGatherer> mGatherer;
     bool mLockedForPrivateBrowsing;
 };
 
 #endif /* _NSPROFILER_H_ */
 
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -210,21 +210,16 @@ PROFILER_FUNC(bool profiler_feature_acti
 // active or not.
 PROFILER_FUNC_VOID(profiler_set_frame_number(int frameNumber))
 
 // Get the profile encoded as a JSON string. A no-op (returning nullptr) if the
 // profiler is inactive.
 PROFILER_FUNC(mozilla::UniquePtr<char[]> profiler_get_profile(double aSinceTime = 0),
               nullptr)
 
-// Get the profile encoded as a JSON object, asynchronously. A no-op if the
-// profiler is inactive.
-PROFILER_FUNC_VOID(profiler_get_profile_jsobject_async(double aSinceTime = 0,
-                                                       mozilla::dom::Promise* = 0))
-
 // Get the params used to start the profiler. Returns 0 and empty vectors (via
 // outparams) if the profile is inactive.
 PROFILER_FUNC_VOID(profiler_get_start_params(int* aEntrySize,
                                              double* aInterval,
                                              mozilla::Vector<const char*>* aFeatures,
                                              mozilla::Vector<const char*>* aFilters))
 
 // Get the profile and write it into a file. A no-op if the profile is
@@ -380,28 +375,16 @@ profiler_call_exit(void* aHandle)
   stack->pop();
 }
 
 // Adds a marker to the PseudoStack. A no-op if the profiler is inactive or in
 // privacy mode.
 void profiler_add_marker(const char *aMarker,
                          ProfilerMarkerPayload *aPayload = nullptr);
 
-// Saves a profile asynchronously. A no-op if the profiler is inactive when the
-// save operation begins.
-MOZ_EXPORT  // XXX: should this be 'extern "C"' as well?
-void profiler_save_profile_to_file_async(double aSinceTime,
-                                         const char* aFileName);
-
-// This function should only be called in response to the observation of a
-// "profiler-subprocess-gather" notification.
-void profiler_will_gather_OOP_profile();
-void profiler_gathered_OOP_profile();
-void profiler_OOP_exit_profile(const nsCString& aProfile);
-
 #define SAMPLER_APPEND_LINE_NUMBER_PASTE(id, line) id ## line
 #define SAMPLER_APPEND_LINE_NUMBER_EXPAND(id, line) SAMPLER_APPEND_LINE_NUMBER_PASTE(id, line)
 #define SAMPLER_APPEND_LINE_NUMBER(id) SAMPLER_APPEND_LINE_NUMBER_EXPAND(id, __LINE__)
 
 // Uncomment this to turn on systrace or build with
 // ac_add_options --enable-systace
 //#define MOZ_USE_SYSTRACE
 #ifdef MOZ_USE_SYSTRACE