Bug 1353630 (part 1) - Refactor ThreadResponsiveness use in ThreadInfo. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 05 Apr 2017 15:53:13 +1000
changeset 400226 6bf903b1e7ccdd88a688b988756e78ddd7014741
parent 400225 a1e5043844f295aa8f9a70517b7b76e34fecc634
child 400227 36e5ca3911b82d759dbc39cd5a5aac10a340585e
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
bugs1353630
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 1353630 (part 1) - Refactor ThreadResponsiveness use in ThreadInfo. r=mstange. Now that ThreadResponsiveness is only used on the main thread, we can refactor ThreadInfo a bit. This patch does the following. - Removes ThreadInfo::mThread, which is unused. - Changes ThreadInfo::mRespInfo to a Maybe<>, and moves the is-main-thread checking outside of ThreadInfo and ThreadResponsiveness. - Renames {ThreadInfo,TickSample}::mRespInfo as mResponsiveness, to better match the class name.
tools/profiler/core/ThreadInfo.cpp
tools/profiler/core/ThreadInfo.h
tools/profiler/core/platform.cpp
tools/profiler/gecko/ThreadResponsiveness.cpp
tools/profiler/gecko/ThreadResponsiveness.h
--- a/tools/profiler/core/ThreadInfo.cpp
+++ b/tools/profiler/core/ThreadInfo.cpp
@@ -28,24 +28,27 @@ ThreadInfo::ThreadInfo(const char* aName
   , mPseudoStack(aPseudoStack)
   , mPlatformData(AllocPlatformData(aThreadId))
   , mStackTop(aStackTop)
   , mPendingDelete(false)
   , mHasProfile(false)
   , mLastSample()
 {
   MOZ_COUNT_CTOR(ThreadInfo);
-  mThread = NS_GetCurrentThread();
 
   // We don't have to guess on mac
 #if defined(GP_OS_darwin)
   pthread_t self = pthread_self();
   mStackTop = pthread_get_stackaddr_np(self);
 #endif
 
+  if (aIsMainThread) {
+    mResponsiveness.emplace();
+  }
+
   // I don't know if we can assert this. But we should warn.
   MOZ_ASSERT(aThreadId >= 0, "native thread ID is < 0");
   MOZ_ASSERT(aThreadId <= INT32_MAX, "native thread ID is > INT32_MAX");
 }
 
 ThreadInfo::~ThreadInfo()
 {
   MOZ_COUNT_DTOR(ThreadInfo);
--- a/tools/profiler/core/ThreadInfo.h
+++ b/tools/profiler/core/ThreadInfo.h
@@ -45,19 +45,16 @@ private:
   const bool mIsMainThread;
 
   // The thread's PseudoStack. This is an owning pointer.
   mozilla::NotNull<PseudoStack*> mPseudoStack;
 
   UniquePlatformData mPlatformData;
   void* mStackTop;
 
-  // May be null for the main thread if the profiler was started during startup.
-  nsCOMPtr<nsIThread> mThread;
-
   // When a thread dies while the profiler is active we keep its ThreadInfo
   // (and its PseudoStack) around for a while, and put it in a "pending delete"
   // state.
   bool mPendingDelete;
 
   //
   // The following code is only used for threads that are being profiled, i.e.
   // for which SetHasProfile() has been called.
@@ -69,34 +66,37 @@ public:
   void StreamJSON(ProfileBuffer* aBuffer, SpliceableJSONWriter& aWriter,
                   const mozilla::TimeStamp& aStartTime, double aSinceTime);
 
   // Call this method when the JS entries inside the buffer are about to
   // become invalid, i.e., just before JS shutdown.
   void FlushSamplesAndMarkers(ProfileBuffer* aBuffer,
                               const mozilla::TimeStamp& aStartTime);
 
-  ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
-
-  void UpdateThreadResponsiveness() {
-    mRespInfo.Update(mIsMainThread, mThread);
+  // Returns nullptr if this is not the main thread.
+  ThreadResponsiveness* GetThreadResponsiveness()
+  {
+    ThreadResponsiveness* responsiveness = mResponsiveness.ptrOr(nullptr);
+    MOZ_ASSERT(!!responsiveness == mIsMainThread);
+    return responsiveness;
   }
 
 private:
   bool mHasProfile;
 
   // JS frames in the buffer may require a live JSRuntime to stream (e.g.,
   // stringifying JIT frames). In the case of JSRuntime destruction,
   // FlushSamplesAndMarkers should be called to save them. These are spliced
   // into the final stream.
   mozilla::UniquePtr<char[]> mSavedStreamedSamples;
   mozilla::UniquePtr<char[]> mSavedStreamedMarkers;
   mozilla::Maybe<UniqueStacks> mUniqueStacks;
 
-  ThreadResponsiveness mRespInfo;
+  // This is only used for the main thread.
+  mozilla::Maybe<ThreadResponsiveness> mResponsiveness;
 
   // When sampling, this holds the generation number and offset in
   // ProfilerState::mBuffer of the most recent sample for this thread.
   ProfileBuffer::LastSample mLastSample;
 };
 
 void
 StreamSamplesAndMarkers(const char* aName, int aThreadId,
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -364,17 +364,17 @@ public:
   TickSample(ThreadInfo* aThreadInfo, int64_t aRSSMemory, int64_t aUSSMemory)
     : mIsSynchronous(false)
     , mTimeStamp(mozilla::TimeStamp::Now())
     , mThreadId(aThreadInfo->ThreadId())
     , mPseudoStack(aThreadInfo->Stack())
     , mStackTop(aThreadInfo->StackTop())
     , mLastSample(&aThreadInfo->LastSample())
     , mPlatformData(aThreadInfo->GetPlatformData())
-    , mRespInfo(aThreadInfo->GetThreadResponsiveness())
+    , mResponsiveness(aThreadInfo->GetThreadResponsiveness())
     , mRSSMemory(aRSSMemory)    // may be zero
     , mUSSMemory(aUSSMemory)    // may be zero
 #if !defined(GP_OS_darwin)
     , mContext(nullptr)
 #endif
     , mPC(nullptr)
     , mSP(nullptr)
     , mFP(nullptr)
@@ -387,17 +387,17 @@ public:
   TickSample(NotNull<PseudoStack*> aPseudoStack, PlatformData* aPlatformData)
     : mIsSynchronous(true)
     , mTimeStamp(mozilla::TimeStamp::Now())
     , mThreadId(Thread::GetCurrentId())
     , mPseudoStack(aPseudoStack)
     , mStackTop(nullptr)
     , mLastSample(nullptr)
     , mPlatformData(aPlatformData)
-    , mRespInfo(nullptr)
+    , mResponsiveness(nullptr)
     , mRSSMemory(0)
     , mUSSMemory(0)
 #if !defined(GP_OS_darwin)
     , mContext(nullptr)
 #endif
     , mPC(nullptr)
     , mSP(nullptr)
     , mFP(nullptr)
@@ -417,17 +417,17 @@ public:
   const NotNull<PseudoStack*> mPseudoStack;
 
   void* const mStackTop;
 
   ProfileBuffer::LastSample* const mLastSample;   // may be null
 
   PlatformData* const mPlatformData;
 
-  ThreadResponsiveness* const mRespInfo;          // may be null
+  ThreadResponsiveness* const mResponsiveness;    // may be null
 
   const int64_t mRSSMemory;                       // may be zero
   const int64_t mUSSMemory;                       // may be zero
 
   // The remaining fields are filled in, after construction, by
   // SamplerThread::SuspendAndSampleAndResume() for periodic samples, and
   // PopulateContext() for synchronous samples. They are filled in separately
   // from the other fields in this class because the code that fills them in is
@@ -1066,19 +1066,19 @@ Tick(PS::LockRef aLock, ProfileBuffer* a
       pseudoStack->getPendingMarkers();
     while (pendingMarkersList && pendingMarkersList->peek()) {
       ProfilerMarker* marker = pendingMarkersList->popHead();
       aBuffer->addStoredMarker(marker);
       aBuffer->addTag(ProfileBufferEntry::Marker(marker));
     }
   }
 
-  if (aSample->mRespInfo && aSample->mRespInfo->HasData()) {
+  if (aSample->mResponsiveness && aSample->mResponsiveness->HasData()) {
     mozilla::TimeDuration delta =
-      aSample->mRespInfo->GetUnresponsiveDuration(aSample->mTimeStamp);
+      aSample->mResponsiveness->GetUnresponsiveDuration(aSample->mTimeStamp);
     aBuffer->addTag(ProfileBufferEntry::Responsiveness(delta.ToMilliseconds()));
   }
 
   // rssMemory is equal to 0 when we are not recording.
   if (aSample->mRSSMemory != 0) {
     double rssMemory = static_cast<double>(aSample->mRSSMemory);
     aBuffer->addTag(ProfileBufferEntry::ResidentMemory(rssMemory));
   }
@@ -1659,17 +1659,20 @@ SamplerThread::Run()
               gPS->Buffer(lock)->DuplicateLastSample(info->ThreadId(),
                                                      gPS->StartTime(lock),
                                                      info->LastSample());
             if (dup_ok) {
               continue;
             }
           }
 
-          info->UpdateThreadResponsiveness();
+          // We only track responsiveness for the main thread.
+          if (info->IsMainThread()) {
+            info->GetThreadResponsiveness()->Update();
+          }
 
           // We only get the memory measurements once for all threads.
           int64_t rssMemory = 0;
           int64_t ussMemory = 0;
           if (i == 0 && gPS->FeatureMemory(lock)) {
             rssMemory = nsMemoryReporterManager::ResidentFast();
 #if defined(GP_OS_linux) || defined(GP_OS_android)
             ussMemory = nsMemoryReporterManager::ResidentUnique();
--- a/tools/profiler/gecko/ThreadResponsiveness.cpp
+++ b/tools/profiler/gecko/ThreadResponsiveness.cpp
@@ -90,22 +90,18 @@ ThreadResponsiveness::~ThreadResponsiven
 {
   MOZ_COUNT_DTOR(ThreadResponsiveness);
   if (mActiveTracerEvent) {
     mActiveTracerEvent->Terminate();
   }
 }
 
 void
-ThreadResponsiveness::Update(bool aIsMainThread, nsIThread* aThread)
+ThreadResponsiveness::Update()
 {
-  if (!aIsMainThread) {
-    return;
-  }
-
   if (!mActiveTracerEvent) {
     mActiveTracerEvent = new CheckResponsivenessTask();
     NS_DispatchToMainThread(mActiveTracerEvent);
   }
 
   mLastTracerTime = mActiveTracerEvent->GetLastTracerTime();
 }
 
--- a/tools/profiler/gecko/ThreadResponsiveness.h
+++ b/tools/profiler/gecko/ThreadResponsiveness.h
@@ -7,24 +7,24 @@
 #define ThreadResponsiveness_h
 
 #include "nsISupports.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/TimeStamp.h"
 
 class CheckResponsivenessTask;
 
+// This class should only be used for the main thread.
 class ThreadResponsiveness {
 public:
   explicit ThreadResponsiveness();
 
   ~ThreadResponsiveness();
 
-  // Won't do anything on non-main threads for now.
-  void Update(bool aIsMainThread, nsIThread* aThread);
+  void Update();
 
   mozilla::TimeDuration GetUnresponsiveDuration(const mozilla::TimeStamp& now) const {
     return now - mLastTracerTime;
   }
 
   bool HasData() const {
     return !mLastTracerTime.IsNull();
   }