Bug 1382910 - Add profiler_ensure_started. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Mon, 24 Jul 2017 16:48:15 -0400
changeset 614735 bfb28d33e32b5690dc7012203248c57f9a1c54bd
parent 614734 6e02c8969b98be9dcf3e0e3b601b782bcd9c5118
child 614736 55808cc575ee9376a7e59ed60ee3585685e5cbe5
push id70096
push userbmo:mstange@themasta.com
push dateMon, 24 Jul 2017 22:17:43 +0000
reviewersnjn
bugs1382910
milestone56.0a1
Bug 1382910 - Add profiler_ensure_started. r?njn MozReview-Commit-ID: LBLlOLXqCwK
tools/profiler/core/platform.cpp
tools/profiler/public/GeckoProfiler.h
tools/profiler/tests/gtest/GeckoProfiler.cpp
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -401,16 +401,35 @@ public:
     delete sInstance;
     sInstance = nullptr;
 
     return samplerThread;
   }
 
   static bool Exists(PSLockRef) { return !!sInstance; }
 
+  static bool Equals(PSLockRef,
+                     int aEntries, double aInterval, uint32_t aFeatures,
+                     const char** aFilters, uint32_t aFilterCount)
+  {
+    if (sInstance->mEntries != aEntries ||
+        sInstance->mInterval != aInterval ||
+        sInstance->mFeatures != aFeatures ||
+        sInstance->mFilters.length() != aFilterCount) {
+      return false;
+    }
+
+    for (uint32_t i = 0; i < sInstance->mFilters.length(); ++i) {
+      if (strcmp(sInstance->mFilters[i].c_str(), aFilters[i]) != 0) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   static size_t SizeOf(PSLockRef, MallocSizeOf aMallocSizeOf)
   {
     size_t n = aMallocSizeOf(sInstance);
 
     n += sInstance->mBuffer->SizeOfIncludingThis(aMallocSizeOf);
 
     return n;
   }
@@ -2628,16 +2647,63 @@ profiler_start(int aEntries, double aInt
     ProfilerParent::ProfilerStopped();
     NotifyObservers("profiler-stopped");
     delete samplerThread;
   }
   NotifyProfilerStarted(aEntries, aInterval, aFeatures,
                         aFilters, aFilterCount);
 }
 
+void
+profiler_ensure_started(int aEntries, double aInterval, uint32_t aFeatures,
+                        const char** aFilters, uint32_t aFilterCount)
+{
+  LOG("profiler_ensure_started");
+
+  bool startedProfiler = false;
+  SamplerThread* samplerThread = nullptr;
+  {
+    PSAutoLock lock(gPSMutex);
+
+    // Initialize if necessary.
+    if (!CorePS::Exists()) {
+      profiler_init(nullptr);
+    }
+
+    if (ActivePS::Exists(lock)) {
+      // The profiler is active.
+      if (!ActivePS::Equals(lock, aEntries, aInterval, aFeatures,
+                            aFilters, aFilterCount)) {
+        // Stop and restart with different settings.
+        samplerThread = locked_profiler_stop(lock);
+        locked_profiler_start(lock, aEntries, aInterval, aFeatures,
+                              aFilters, aFilterCount);
+        startedProfiler = true;
+      }
+    } else {
+      // The profiler is stopped.
+      locked_profiler_start(lock, aEntries, aInterval, aFeatures,
+                            aFilters, aFilterCount);
+      startedProfiler = true;
+    }
+  }
+
+  // We do these operations with gPSMutex unlocked. The comments in
+  // profiler_stop() explain why.
+  if (samplerThread) {
+    ProfilerParent::ProfilerStopped();
+    NotifyObservers("profiler-stopped");
+    delete samplerThread;
+  }
+  if (startedProfiler) {
+    NotifyProfilerStarted(aEntries, aInterval, aFeatures,
+                          aFilters, aFilterCount);
+  }
+}
+
 static MOZ_MUST_USE SamplerThread*
 locked_profiler_stop(PSLockRef aLock)
 {
   LOG("locked_profiler_stop");
 
   MOZ_RELEASE_ASSERT(CorePS::Exists() && ActivePS::Exists(aLock));
 
   // At the very start, clear RacyFeatures.
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -156,16 +156,26 @@ PROFILER_FUNC_VOID(profiler_shutdown())
 PROFILER_FUNC_VOID(profiler_start(int aEntries, double aInterval,
                                   uint32_t aFeatures,
                                   const char** aFilters, uint32_t aFilterCount))
 
 // Stop the profiler and discard the profile without saving it. A no-op if the
 // profiler is inactive. After stopping the profiler is "inactive".
 PROFILER_FUNC_VOID(profiler_stop())
 
+// If the profiler is inactive, start it. If it's already active, restart it if
+// the requested settings differ from the current settings. Both the check and
+// the state change are performed while the profiler state is locked.
+// The only difference to profiler_start is that the current buffer contents are
+// not discarded if the profiler is already running with the requested settings.
+PROFILER_FUNC_VOID(profiler_ensure_started(int aEntries, double aInterval,
+                                           uint32_t aFeatures,
+                                           const char** aFilters,
+                                           uint32_t aFilterCount))
+
 //---------------------------------------------------------------------------
 // Control the profiler
 //---------------------------------------------------------------------------
 
 // Register/unregister threads with the profiler. Both functions operate the
 // same whether the profiler is active or inactive.
 PROFILER_FUNC_VOID(profiler_register_thread(const char* name,
                                             void* guessStackTop))
--- a/tools/profiler/tests/gtest/GeckoProfiler.cpp
+++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -166,16 +166,89 @@ TEST(GeckoProfiler, FeaturesAndParams)
     // These calls are no-ops.
     profiler_stop();
     profiler_stop();
 
     InactiveFeaturesAndParamsCheck();
   }
 }
 
+TEST(GeckoProfiler, EnsureStarted)
+{
+  InactiveFeaturesAndParamsCheck();
+
+  uint32_t features = ProfilerFeature::JS | ProfilerFeature::Threads;
+  const char* filters[] = { "GeckoMain", "Compositor" };
+  {
+    // Inactive -> Active
+    profiler_ensure_started(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                            features, filters, MOZ_ARRAY_LENGTH(filters));
+
+    ActiveParamsCheck(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                      features, filters, MOZ_ARRAY_LENGTH(filters));
+  }
+
+  {
+    // Active -> Active with same settings
+
+    // First, write some samples into the buffer.
+    PR_Sleep(PR_MillisecondsToInterval(500));
+
+    uint32_t currPos1, entries1, generation1;
+    profiler_get_buffer_info(&currPos1, &entries1, &generation1);
+    ASSERT_TRUE(generation1 > 0 || currPos1 > 0);
+
+    // Call profiler_ensure_started with the same settings as before.
+    // This operation must not clear our buffer!
+    profiler_ensure_started(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                            features, filters, MOZ_ARRAY_LENGTH(filters));
+
+    ActiveParamsCheck(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                      features, filters, MOZ_ARRAY_LENGTH(filters));
+
+    // Check that our position in the buffer stayed the same or advanced.
+    // In particular, it shouldn't have reverted to the start.
+    uint32_t currPos2, entries2, generation2;
+    profiler_get_buffer_info(&currPos2, &entries2, &generation2);
+    ASSERT_TRUE(generation2 >= generation1);
+    ASSERT_TRUE(generation2 > generation1 || currPos2 >= currPos1);
+  }
+
+  {
+    // Active -> Active with *different* settings
+
+    uint32_t currPos1, entries1, generation1;
+    profiler_get_buffer_info(&currPos1, &entries1, &generation1);
+
+    // Call profiler_ensure_started with a different feature set than the one it's
+    // currently running with. This is supposed to stop and restart the
+    // profiler, thereby discarding the buffer contents.
+    uint32_t differentFeatures = features | ProfilerFeature::Leaf;
+    profiler_ensure_started(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                            differentFeatures,
+                            filters, MOZ_ARRAY_LENGTH(filters));
+
+    ActiveParamsCheck(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
+                      differentFeatures, filters, MOZ_ARRAY_LENGTH(filters));
+
+    uint32_t currPos2, entries2, generation2;
+    profiler_get_buffer_info(&currPos2, &entries2, &generation2);
+    ASSERT_TRUE(generation2 <= generation1);
+    ASSERT_TRUE(generation2 < generation1 || currPos2 < currPos1);
+  }
+
+  {
+    // Active -> Inactive
+
+    profiler_stop();
+
+    InactiveFeaturesAndParamsCheck();
+  }
+}
+
 TEST(GeckoProfiler, DifferentThreads)
 {
   InactiveFeaturesAndParamsCheck();
 
   nsCOMPtr<nsIThread> thread;
   nsresult rv = NS_NewNamedThread("GeckoProfGTest", getter_AddRefs(thread));
   ASSERT_TRUE(NS_SUCCEEDED(rv));