Bug 1447338 - Add thread "responsiveness" as a configurable feature to the Gecko Profiler; r=mstange draft
authorGreg Tatum <gtatum@mozilla.com>
Wed, 28 Mar 2018 15:28:54 -0500
changeset 796903 bac38dc65bf074f31b2e25f80620a6baa4c7e14d
parent 796393 24bae072acb09114c367e6b9ffde9261b2ad8a58
push id110386
push usergtatum@mozilla.com
push dateFri, 18 May 2018 14:43:43 +0000
reviewersmstange
bugs1447338
milestone62.0a1
Bug 1447338 - Add thread "responsiveness" as a configurable feature to the Gecko Profiler; r=mstange MozReview-Commit-ID: KTJRvQzUwsf
browser/components/extensions/schemas/geckoProfiler.json
devtools/client/performance-new/components/Settings.js
devtools/client/performance-new/store/reducers.js
devtools/client/performance-new/test/chrome/test_perf-settings-features.html
devtools/server/actors/perf.js
tools/profiler/core/ProfiledThreadData.cpp
tools/profiler/core/ProfiledThreadData.h
tools/profiler/core/platform.cpp
tools/profiler/public/GeckoProfiler.h
--- a/browser/components/extensions/schemas/geckoProfiler.json
+++ b/browser/components/extensions/schemas/geckoProfiler.json
@@ -24,16 +24,17 @@
         "type": "string",
         "enum": [
           "java",
           "js",
           "leaf",
           "mainthreadio",
           "memory",
           "privacy",
+          "responsiveness",
           "restyle",
           "screenshots",
           "stackwalk",
           "tasktracer",
           "threads",
           "trackopts"
         ]
       }
--- a/devtools/client/performance-new/components/Settings.js
+++ b/devtools/client/performance-new/components/Settings.js
@@ -91,16 +91,22 @@ const featureCheckboxes = [
   },
   {
     name: "JavaScript",
     value: "js",
     title: "Record JavaScript stack information, and interleave it with native stacks.",
     recommended: true
   },
   {
+    name: "Responsiveness",
+    value: "responsiveness",
+    title: "Collect thread responsiveness information.",
+    recommended: true
+  },
+  {
     name: "Java",
     value: "java",
     title: "Profile Java code (Android only)."
   },
   {
     name: "Native Leaf Stack",
     value: "leaf",
     title: "Record the native memory address of the leaf-most stack. This could be " +
--- a/devtools/client/performance-new/store/reducers.js
+++ b/devtools/client/performance-new/store/reducers.js
@@ -83,17 +83,17 @@ function entries(state = 10000000, actio
       return state;
   }
 }
 
 /**
  * The features that are enabled for the profiler.
  * @param {array} state
  */
-function features(state = ["js", "stackwalk"], action) {
+function features(state = ["js", "stackwalk", "responsiveness"], action) {
   switch (action.type) {
     case "CHANGE_FEATURES":
       return action.features;
     case "INITIALIZE_STORE":
       return action.recordingSettingsFromPreferences.features;
     default:
       return state;
   }
--- a/devtools/client/performance-new/test/chrome/test_perf-settings-features.html
+++ b/devtools/client/performance-new/test/chrome/test_perf-settings-features.html
@@ -29,42 +29,43 @@
           recordingPreferencesCalls
         } = createPerfComponent();
 
         await mountAndInitializeComponent();
 
         // Open up the features summary.
         document.querySelector("#perf-settings-features-summary").click();
 
-        is(selectors.getFeatures(getState()).join(","), "js,stackwalk",
+        is(selectors.getFeatures(getState()).join(","), "js,stackwalk,responsiveness",
           "The features starts out with the default");
         is(recordingPreferencesCalls.length, 0,
           "No calls have been made to set preferences");
 
         // Click the "features checkbox.
         document.querySelector("#perf-settings-feature-checkbox-js").click();
 
-        is(selectors.getFeatures(getState()).join(","), "stackwalk",
+        is(selectors.getFeatures(getState()).join(","), "stackwalk,responsiveness",
           "The feature has been removed.");
         is(recordingPreferencesCalls.length, 1,
           "The preferences have been updated.");
-        is(recordingPreferencesCalls[0].features.join(","), "stackwalk",
+        is(recordingPreferencesCalls[0].features.join(","), "stackwalk,responsiveness",
           "The preferences have been updated.");
 
         // Enable a feature
         document.querySelector("#perf-settings-feature-checkbox-leaf").click();
-        is(selectors.getFeatures(getState()).join(","), "leaf,stackwalk",
+        is(selectors.getFeatures(getState()).join(","), "leaf,stackwalk,responsiveness",
           "Another feature was added");
 
         // Start the profiler by clicking the start button, and flushing the async
         // calls out to the mock perf front.
         document.querySelector("button").click();
         await perfFrontMock._flushAsyncQueue();
 
         is(perfFrontMock._startProfilerCalls.length, 1,
           "Start profiler was called once");
-        is(perfFrontMock._startProfilerCalls[0].features.join(","), "leaf,stackwalk",
+        is(perfFrontMock._startProfilerCalls[0].features.join(","),
+          "leaf,stackwalk,responsiveness",
           "Start profiler was called with the correct features");
       });
     </script>
   </pre>
 </body>
 </html>
--- a/devtools/server/actors/perf.js
+++ b/devtools/server/actors/perf.js
@@ -50,17 +50,18 @@ exports.PerfActor = ActorClassWithSpec(p
       return false;
     }
 
     // For a quick implementation, decide on some default values. These may need
     // to be tweaked or made configurable as needed.
     const settings = {
       entries: options.entries || 1000000,
       interval: options.interval || 1,
-      features: options.features || ["js", "stackwalk", "threads", "leaf"],
+      features: options.features ||
+        ["js", "stackwalk", "responsiveness", "threads", "leaf"],
       threads: options.threads || ["GeckoMain", "Compositor"]
     };
 
     try {
       // This can throw an error if the profiler is in the wrong state.
       Services.profiler.StartProfiler(
         settings.entries,
         settings.interval,
--- a/tools/profiler/core/ProfiledThreadData.cpp
+++ b/tools/profiler/core/ProfiledThreadData.cpp
@@ -13,21 +13,24 @@
 #ifdef XP_WIN
 #include <process.h>
 #define getpid _getpid
 #else
 #include <unistd.h> // for getpid()
 #endif
 
 ProfiledThreadData::ProfiledThreadData(ThreadInfo* aThreadInfo,
-                                       nsIEventTarget* aEventTarget)
+                                       nsIEventTarget* aEventTarget,
+                                       bool aIncludeResponsiveness)
   : mThreadInfo(aThreadInfo)
 {
   MOZ_COUNT_CTOR(ProfiledThreadData);
-  mResponsiveness.emplace(aEventTarget, aThreadInfo->IsMainThread());
+  if (aIncludeResponsiveness) {
+    mResponsiveness.emplace(aEventTarget, aThreadInfo->IsMainThread());
+  }
 }
 
 ProfiledThreadData::~ProfiledThreadData()
 {
   MOZ_COUNT_DTOR(ProfiledThreadData);
 }
 
 void
--- a/tools/profiler/core/ProfiledThreadData.h
+++ b/tools/profiler/core/ProfiledThreadData.h
@@ -37,17 +37,18 @@
 // If that happens, NotifyUnregistered() is called.
 //
 // This class is the right place to store buffer positions. Profiler buffer
 // positions become invalid if the profiler buffer is destroyed, which happens
 // when the profiler is stopped.
 class ProfiledThreadData final
 {
 public:
-  ProfiledThreadData(ThreadInfo* aThreadInfo, nsIEventTarget* aEventTarget);
+  ProfiledThreadData(ThreadInfo* aThreadInfo, nsIEventTarget* aEventTarget,
+                     bool aIncludeResponsiveness);
   ~ProfiledThreadData();
 
   void NotifyUnregistered(uint64_t aBufferPosition)
   {
     mResponsiveness.reset();
     mLastSample = mozilla::Nothing();
     MOZ_ASSERT(!mBufferPositionWhenReceivedJSContext,
                "JSContext should have been cleared before the thread was unregistered");
@@ -58,18 +59,18 @@ public:
 
   mozilla::Maybe<uint64_t>& LastSample() { return mLastSample; }
 
   void StreamJSON(const ProfileBuffer& aBuffer, JSContext* aCx,
                   SpliceableJSONWriter& aWriter,
                   const mozilla::TimeStamp& aProcessStartTime,
                   double aSinceTime);
 
-  // Returns nullptr if this is not the main thread or if this thread is not
-  // being profiled.
+  // Returns nullptr if this is not the main thread, the responsiveness
+  // feature is not turned on, or if this thread is not being profiled.
   ThreadResponsiveness* GetThreadResponsiveness()
   {
     ThreadResponsiveness* responsiveness = mResponsiveness.ptrOr(nullptr);
     return responsiveness;
   }
 
   const RefPtr<ThreadInfo> Info() const { return mThreadInfo; }
 
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -1789,17 +1789,18 @@ locked_profiler_stream_json_for_this_pro
      java::GeckoJavaSampler::Pause();
 
      UniquePtr<ProfileBuffer> javaBuffer = CollectJavaThreadProfileData();
 
      // Thread id of java Main thread is 0, if we support profiling of other
      // java thread, we have to get thread id and name via JNI.
      RefPtr<ThreadInfo> threadInfo =
        new ThreadInfo("Java Main Thread", 0, false);
-     ProfiledThreadData profiledThreadData(threadInfo, nullptr);
+     ProfiledThreadData profiledThreadData(threadInfo, nullptr,
+                                           ActivePS::FeatureResponsiveness(aLock));
      profiledThreadData.StreamJSON(*javaBuffer.get(), nullptr, aWriter,
                                    CorePS::ProcessStartTime(), aSinceTime);
 
      java::GeckoJavaSampler::Unpause();
   }
 #endif
 
   }
@@ -2084,17 +2085,20 @@ SamplerThread::Run()
               ActivePS::Buffer(lock).DuplicateLastSample(
                 info->ThreadId(), CorePS::ProcessStartTime(),
                 profiledThreadData->LastSample());
             if (dup_ok) {
               continue;
             }
           }
 
-          profiledThreadData->GetThreadResponsiveness()->Update();
+          ThreadResponsiveness* resp = profiledThreadData->GetThreadResponsiveness();
+          if (resp) {
+            resp->Update();
+          }
 
           TimeStamp now = TimeStamp::Now();
           SuspendAndSampleAndResumeThread(lock, *registeredThread,
                                           [&](const Registers& aRegs) {
             DoPeriodicSample(lock, *registeredThread, *profiledThreadData, now,
                              aRegs, rssMemory, ussMemory);
           });
         }
@@ -2264,17 +2268,18 @@ locked_register_thread(PSLockRef aLock, 
 
   TLSRegisteredThread::SetRegisteredThread(aLock, registeredThread.get());
 
   if (ActivePS::Exists(aLock) &&
       ActivePS::ShouldProfileThread(aLock, info)) {
     nsCOMPtr<nsIEventTarget> eventTarget = registeredThread->GetEventTarget();
     ProfiledThreadData* profiledThreadData =
       ActivePS::AddLiveProfiledThread(aLock, registeredThread.get(),
-        MakeUnique<ProfiledThreadData>(info, eventTarget));
+        MakeUnique<ProfiledThreadData>(info, eventTarget,
+                                       ActivePS::FeatureResponsiveness(aLock)));
 
     if (ActivePS::FeatureJS(aLock)) {
       // This StartJSSampling() call is on-thread, so we can poll manually to
       // start JS sampling immediately.
       registeredThread->StartJSSampling(
         ActivePS::FeatureTrackOptimizations(aLock));
       registeredThread->PollJSSampling();
       if (registeredThread->GetJSContext()) {
@@ -2866,17 +2871,18 @@ locked_profiler_start(PSLockRef aLock, u
     CorePS::RegisteredThreads(aLock);
   for (auto& registeredThread : registeredThreads) {
     RefPtr<ThreadInfo> info = registeredThread->Info();
 
     if (ActivePS::ShouldProfileThread(aLock, info)) {
       nsCOMPtr<nsIEventTarget> eventTarget = registeredThread->GetEventTarget();
       ProfiledThreadData* profiledThreadData =
         ActivePS::AddLiveProfiledThread(aLock, registeredThread.get(),
-          MakeUnique<ProfiledThreadData>(info, eventTarget));
+          MakeUnique<ProfiledThreadData>(info, eventTarget,
+                                         ActivePS::FeatureResponsiveness(aLock)));
       if (ActivePS::FeatureJS(aLock)) {
         registeredThread->StartJSSampling(
           ActivePS::FeatureTrackOptimizations(aLock));
         if (info->ThreadId() == tid) {
           // We can manually poll the current thread so it starts sampling
           // immediately.
           registeredThread->PollJSSampling();
         } else if (info->IsMainThread()) {
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -117,18 +117,21 @@ class TimeStamp;
   macro(3, "mainthreadio", MainThreadIO) \
   \
   /* Add memory measurements (e.g. RSS). */ \
   macro(4, "memory", Memory) \
   \
   /* Do not include user-identifiable information. */ \
   macro(5, "privacy", Privacy) \
   \
+  /* Collect thread responsiveness information. */ \
+  macro(6, "responsiveness", Responsiveness) \
+  \
   /* Restyle profiling. */ \
-  macro(6, "restyle", Restyle) \
+  macro(7, "restyle", Restyle) \
   \
   /* Take a snapshot of the window on every composition. */ \
   macro(7, "screenshots", Screenshots) \
   \
   /* Walk the C++ stack. Not available on all platforms. */ \
   macro(8, "stackwalk", StackWalk) \
   \
   /* Start profiling with feature TaskTracer. */ \