Bug 1321907 - Remove mIsProfilerActive. r=njn
authorMarkus Stange <mstange@themasta.com>
Wed, 22 Mar 2017 21:45:10 -0400
changeset 348981 33e07f746b5e6b0c5a9416010d42f973d55dba9e
parent 348980 e6086e949850dd5434860e50bb75001bec004a03
child 348982 e9043c051769d400fed4c6e3406ca417ddb8a79c
push id39363
push usermstange@themasta.com
push dateThu, 23 Mar 2017 02:44:54 +0000
treeherderautoland@e9043c051769 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1321907
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 1321907 - Remove mIsProfilerActive. r=njn Replace it with profiler_is_active() in one place, and simply remove it in the other places. These other places are: - Around the call to profiler_OOP_exit_profile: profiler_OOP_exit_profile itself already checks whether the profiler is running and does nothing if it's not. - When handling the 'profiler-subprocess-gather' notification. This notification is sent by the profiler because it's interested in the profile, so there's little reason to reject it. - In RecvProfile: If the child process sent us a profile, it did so in response to a GatherProfile request, so chances are that we're still interested in that response. These changes may get us a little closer to a state where you can call getProfileDataAsync, stop the profiler before the content process profiles have all come in, and then still receive a response with all the profiles. At the moment, stopping the profiler will abort the profile gathering process, but that seems more like an accident and less like the behavior you'd want. MozReview-Commit-ID: 2tRXC70BztJ
tools/profiler/gecko/CrossProcessProfilerController.cpp
tools/profiler/public/CrossProcessProfilerController.h
--- a/tools/profiler/gecko/CrossProcessProfilerController.cpp
+++ b/tools/profiler/gecko/CrossProcessProfilerController.cpp
@@ -56,43 +56,40 @@ private:
 };
 
 NS_IMPL_ISUPPORTS(ProfilerObserver, nsIObserver)
 
 CrossProcessProfilerController::CrossProcessProfilerController(
   ProfilerControllingProcess* aProcess)
   : mProcess(aProcess)
   , mObserver(new ProfilerObserver(*this))
-  , mIsProfilerActive(false)
 {
-  nsCOMPtr<nsIProfiler> profiler(do_GetService("@mozilla.org/tools/profiler;1"));
-  bool profilerActive = false;
-  DebugOnly<nsresult> rv = profiler->IsActive(&profilerActive);
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
-
-  if (profilerActive) {
+  if (profiler_is_active()) {
+    // If the profiler is already running in this process, start it in the
+    // child process immediately.
     nsCOMPtr<nsIProfilerStartParams> currentProfilerParams;
-    rv = profiler->GetStartParams(getter_AddRefs(currentProfilerParams));
+    nsCOMPtr<nsIProfiler> profiler(do_GetService("@mozilla.org/tools/profiler;1"));
+    DebugOnly<nsresult> rv = profiler->GetStartParams(getter_AddRefs(currentProfilerParams));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
 
     StartProfiler(currentProfilerParams);
   }
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->AddObserver(mObserver, sObserverTopics[i], false);
     }
   }
 }
 
 CrossProcessProfilerController::~CrossProcessProfilerController()
 {
-  if (mIsProfilerActive && !mProfile.IsEmpty()) {
+  if (!mProfile.IsEmpty()) {
     profiler_OOP_exit_profile(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]);
@@ -111,33 +108,29 @@ CrossProcessProfilerController::StartPro
 
   ipcParams.enabled() = true;
   aParams->GetEntries(&ipcParams.entries());
   aParams->GetInterval(&ipcParams.interval());
   ipcParams.features() = aParams->GetFeatures();
   ipcParams.threadFilters() = aParams->GetThreadFilterNames();
 
   mProcess->SendStartProfiler(ipcParams);
-
-  mIsProfilerActive = true;
 }
 
 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.
-    if (mIsProfilerActive) {
-      profiler_will_gather_OOP_profile();
-      mProcess->SendGatherProfile();
-    }
+    profiler_will_gather_OOP_profile();
+    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);
     if (pse) {
@@ -150,35 +143,31 @@ CrossProcessProfilerController::Observe(
   // These four notifications are sent by the profiler when its corresponding
   // methods are called inside this process. These state changes just need to
   // be forwarded to the other process.
   else if (!strcmp(aTopic, "profiler-started")) {
     nsCOMPtr<nsIProfilerStartParams> params(do_QueryInterface(aSubject));
     StartProfiler(params);
   }
   else if (!strcmp(aTopic, "profiler-stopped")) {
-    mIsProfilerActive = false;
     mProcess->SendStopProfiler();
   }
   else if (!strcmp(aTopic, "profiler-paused")) {
     mProcess->SendPauseProfiler(true);
   }
   else if (!strcmp(aTopic, "profiler-resumed")) {
     mProcess->SendPauseProfiler(false);
   }
 }
 
 // This is called in response to a SendGatherProfile request, or when the
 // other process exits while the profiler is running.
 void
 CrossProcessProfilerController::RecvProfile(const nsCString& aProfile)
 {
-  if (NS_WARN_IF(!mIsProfilerActive)) {
-    return;
-  }
   // 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();
 }
--- a/tools/profiler/public/CrossProcessProfilerController.h
+++ b/tools/profiler/public/CrossProcessProfilerController.h
@@ -31,14 +31,13 @@ private:
   void StartProfiler(nsIProfilerStartParams* aParams);
   void Observe(nsISupports* aSubject, const char* aTopic);
 
   friend class ProfilerObserver;
 
   ProfilerControllingProcess* mProcess;
   RefPtr<ProfilerObserver> mObserver;
   nsCString mProfile;
-  bool mIsProfilerActive;
 };
 
 } // namespace mozilla
 
 #endif