Bug 1433583 - Discard information about old dead threads that no longer have any samples in the buffer. r?mystor draft
authorMarkus Stange <mstange@themasta.com>
Tue, 30 Jan 2018 16:00:29 -0500
changeset 752191 78f5649e38283765a7266a0da5f245d07af6316d
parent 752153 65133e49fbfd5306632301f74be7cd15890bdf9f
push id98193
push userbmo:mstange@themasta.com
push dateWed, 07 Feb 2018 17:55:10 +0000
reviewersmystor
bugs1433583
milestone60.0a1
Bug 1433583 - Discard information about old dead threads that no longer have any samples in the buffer. r?mystor MozReview-Commit-ID: 5ThtN1H1ieA
tools/profiler/core/ThreadInfo.h
tools/profiler/core/platform.cpp
--- a/tools/profiler/core/ThreadInfo.h
+++ b/tools/profiler/core/ThreadInfo.h
@@ -188,29 +188,35 @@ public:
   bool IsMainThread() const { return mIsMainThread; }
 
   mozilla::NotNull<RacyThreadInfo*> RacyInfo() const { return mRacyInfo; }
 
   void StartProfiling();
   void StopProfiling();
   bool IsBeingProfiled() { return mIsBeingProfiled; }
 
-  void NotifyUnregistered() { mUnregisterTime = TimeStamp::Now(); }
+  void NotifyUnregistered(uint64_t aBufferPosition)
+  {
+    mUnregisterTime = TimeStamp::Now();
+    mBufferPositionWhenUnregistered = mozilla::Some(aBufferPosition);
+  }
+  mozilla::Maybe<uint64_t> BufferPositionWhenUnregistered() { return mBufferPositionWhenUnregistered; }
 
   PlatformData* GetPlatformData() const { return mPlatformData.get(); }
   void* StackTop() const { return mStackTop; }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   mozilla::Maybe<uint64_t>& LastSample() { return mLastSample; }
 
 private:
   mozilla::UniqueFreePtr<char> mName;
   mozilla::TimeStamp mRegisterTime;
   mozilla::TimeStamp mUnregisterTime;
+  mozilla::Maybe<uint64_t> mBufferPositionWhenUnregistered;
   const bool mIsMainThread;
   nsCOMPtr<nsIEventTarget> mThread;
 
   // The thread's RacyThreadInfo. This is an owning pointer. It could be an
   // inline member, but we don't do that because RacyThreadInfo is quite large
   // (due to the PseudoStack within it), and we have ThreadInfo vectors and so
   // we'd end up wasting a lot of space in those vectors for excess elements.
   mozilla::NotNull<RacyThreadInfo*> mRacyInfo;
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -279,16 +279,32 @@ public:
   }
 
   // No PSLockRef is needed for this field because it's immutable.
   PS_GET_LOCKLESS(TimeStamp, ProcessStartTime)
 
   PS_GET(ThreadVector&, LiveThreads)
   PS_GET(ThreadVector&, DeadThreads)
 
+  static void DiscardExpiredDeadThreads(PSLockRef, uint64_t aBufferRangeStart)
+  {
+    // Discard any dead threads that were unregistered before aBufferRangeStart.
+    ThreadVector& deadThreads = sInstance->mDeadThreads;
+    for (size_t i = 0; i < deadThreads.size(); i++) {
+      Maybe<uint64_t> bufferPosition =
+        deadThreads.at(i)->BufferPositionWhenUnregistered();
+      MOZ_RELEASE_ASSERT(bufferPosition, "should have unregistered this thread");
+      if (*bufferPosition < aBufferRangeStart) {
+        delete deadThreads.at(i);
+        deadThreads.erase(deadThreads.begin() + i);
+        i--;
+      }
+    }
+  }
+
 #ifdef USE_LUL_STACKWALK
   static lul::LUL* Lul(PSLockRef) { return sInstance->mLul.get(); }
   static void SetLul(PSLockRef, UniquePtr<lul::LUL> aLul)
   {
     sInstance->mLul = Move(aLul);
   }
 #endif
 
@@ -1438,16 +1454,17 @@ StreamTaskTracer(PSLockRef aLock, Splice
   aWriter.StartArrayProperty("threads");
   {
     const CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(aLock);
     for (size_t i = 0; i < liveThreads.size(); i++) {
       ThreadInfo* info = liveThreads.at(i);
       StreamNameAndThreadId(aWriter, info->Name(), info->ThreadId());
     }
 
+    CorePS::DiscardExpiredDeadThreads(aLock, ActivePS::Buffer(aLock).mRangeStart);
     const CorePS::ThreadVector& deadThreads = CorePS::DeadThreads(aLock);
     for (size_t i = 0; i < deadThreads.size(); i++) {
       ThreadInfo* info = deadThreads.at(i);
       StreamNameAndThreadId(aWriter, info->Name(), info->ThreadId());
     }
   }
   aWriter.EndArray();
 
@@ -1683,16 +1700,17 @@ locked_profiler_stream_json_for_this_pro
     for (size_t i = 0; i < liveThreads.size(); i++) {
       ThreadInfo* info = liveThreads.at(i);
       if (!info->IsBeingProfiled()) {
         continue;
       }
       info->StreamJSON(buffer, aWriter, CorePS::ProcessStartTime(), aSinceTime);
     }
 
+    CorePS::DiscardExpiredDeadThreads(aLock, ActivePS::Buffer(aLock).mRangeStart);
     const CorePS::ThreadVector& deadThreads = CorePS::DeadThreads(aLock);
     for (size_t i = 0; i < deadThreads.size(); i++) {
       ThreadInfo* info = deadThreads.at(i);
       MOZ_ASSERT(info->IsBeingProfiled());
       info->StreamJSON(buffer, aWriter, CorePS::ProcessStartTime(), aSinceTime);
     }
 
 #if defined(GP_OS_android)
@@ -3090,18 +3108,19 @@ profiler_unregister_thread()
   // that for a JS thread that is in the process of disappearing.
 
   int i;
   ThreadInfo* info = FindLiveThreadInfo(lock, &i);
   MOZ_RELEASE_ASSERT(info == TLSInfo::Info(lock));
   if (info) {
     DEBUG_LOG("profiler_unregister_thread: %s", info->Name());
     if (ActivePS::Exists(lock) && info->IsBeingProfiled()) {
-      info->NotifyUnregistered();
+      info->NotifyUnregistered(ActivePS::Buffer(lock).mRangeEnd);
       CorePS::DeadThreads(lock).push_back(info);
+      CorePS::DiscardExpiredDeadThreads(lock, ActivePS::Buffer(lock).mRangeStart);
     } else {
       delete info;
     }
     CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(lock);
     liveThreads.erase(liveThreads.begin() + i);
 
     // Whether or not we just destroyed the ThreadInfo or transferred it to the
     // dead thread vector, we no longer need to access it via TLS.