Bug 1054110 - Let profiling circular buffer survive thread shutdown. r=ehsan
authorBenoit Girard <b56girard@gmail.com>
Mon, 06 Oct 2014 14:12:52 -0400
changeset 233119 24ffba42510dae9ee20a80849772349b9a1ddf1b
parent 233118 3e779f1cf1d9c24484844184cbe659cf50d8326f
child 233120 57a53c8b472ea9edf22aafb6d39b0813e75fabcc
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1054110
milestone35.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 1054110 - Let profiling circular buffer survive thread shutdown. r=ehsan
tools/profiler/ProfileEntry.h
tools/profiler/TableTicker.cpp
tools/profiler/TableTicker.h
tools/profiler/platform-linux.cc
tools/profiler/platform-macos.cc
tools/profiler/platform-win32.cc
tools/profiler/platform.cpp
tools/profiler/platform.h
tools/profiler/tests/gtest/ThreadProfileTest.cpp
--- a/tools/profiler/ProfileEntry.h
+++ b/tools/profiler/ProfileEntry.h
@@ -101,16 +101,21 @@ public:
   bool HasGenerationExpired(int aGenID) const {
     return aGenID + 2 <= mGeneration;
   }
   void* GetStackTop() const { return mStackTop; }
   void DuplicateLastSample();
 
   ThreadInfo* GetThreadInfo() const { return mThreadInfo; }
   ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
+  void SetPendingDelete()
+  {
+    mPseudoStack = nullptr;
+    mPlatformData = nullptr;
+  }
 private:
   FRIEND_TEST(ThreadProfile, InsertOneTag);
   FRIEND_TEST(ThreadProfile, InsertOneTagWithTinyBuffer);
   FRIEND_TEST(ThreadProfile, InsertTagsNoWrap);
   FRIEND_TEST(ThreadProfile, InsertTagsWrap);
   FRIEND_TEST(ThreadProfile, MemoryMeasure);
   ThreadInfo* mThreadInfo;
   // Circular buffer 'Keep One Slot Open' implementation
--- a/tools/profiler/TableTicker.cpp
+++ b/tools/profiler/TableTicker.cpp
@@ -279,16 +279,19 @@ void TableTicker::StreamJSObject(JSStrea
       {
         mozilla::MutexAutoLock lock(*sRegisteredThreadsMutex);
 
         for (size_t i = 0; i < sRegisteredThreads->size(); i++) {
           // Thread not being profiled, skip it
           if (!sRegisteredThreads->at(i)->Profile())
             continue;
 
+          // Note that we intentionally include ThreadProfile which
+          // have been marked for pending delete.
+
           MutexAutoLock lock(*sRegisteredThreads->at(i)->Profile()->GetMutex());
 
           sRegisteredThreads->at(i)->Profile()->StreamJSObject(b);
         }
       }
 
       if (Sampler::CanNotifyObservers()) {
         // Send a event asking any subprocesses (plugins) to
--- a/tools/profiler/TableTicker.h
+++ b/tools/profiler/TableTicker.h
@@ -115,16 +115,23 @@ class TableTicker: public Sampler {
 
       for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
         ThreadInfo* info = sRegisteredThreads->at(i);
         ThreadProfile* profile = info->Profile();
         if (profile) {
           delete profile;
           info->SetProfile(nullptr);
         }
+        // We've stopped profiling. We no longer need to retain
+        // information for an old thread.
+        if (info->IsPendingDelete()) {
+          delete info;
+          sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
+          i--;
+        }
       }
     }
 #if defined(XP_WIN)
     delete mIntelPowerGadget;
 #endif
   }
 
   void RegisterThread(ThreadInfo* aInfo) {
@@ -156,17 +163,17 @@ class TableTicker: public Sampler {
 
   ThreadProfile* GetPrimaryThreadProfile()
   {
     if (!mPrimaryThreadProfile) {
       mozilla::MutexAutoLock lock(*sRegisteredThreadsMutex);
 
       for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
         ThreadInfo* info = sRegisteredThreads->at(i);
-        if (info->IsMainThread()) {
+        if (info->IsMainThread() && !info->IsPendingDelete()) {
           mPrimaryThreadProfile = info->Profile();
           break;
         }
       }
     }
 
     return mPrimaryThreadProfile;
   }
--- a/tools/profiler/platform-linux.cc
+++ b/tools/profiler/platform-linux.cc
@@ -306,17 +306,17 @@ static void* SignalSender(void* arg) {
       std::vector<ThreadInfo*> threads =
         SamplerRegistry::sampler->GetRegisteredThreads();
 
       bool isFirstProfiledThread = true;
       for (uint32_t i = 0; i < threads.size(); i++) {
         ThreadInfo* info = threads[i];
 
         // This will be null if we're not interested in profiling this thread.
-        if (!info->Profile())
+        if (!info->Profile() || info->IsPendingDelete())
           continue;
 
         PseudoStack::SleepState sleeping = info->Stack()->observeSleeping();
         if (sleeping == PseudoStack::SLEEPING_AGAIN) {
           info->Profile()->DuplicateLastSample();
           //XXX: This causes flushes regardless of jank-only mode
           info->Profile()->flush();
           continue;
@@ -454,17 +454,17 @@ bool Sampler::RegisterCurrentThread(cons
   if (!Sampler::sRegisteredThreadsMutex)
     return false;
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = gettid();
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
       // Thread already registered. This means the first unregister will be
       // too early.
       ASSERT(false);
       return false;
     }
   }
 
   set_tls_stack_top(stackTop);
@@ -490,20 +490,28 @@ void Sampler::UnregisterCurrentThread()
   tlsStackTop.set(nullptr);
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = gettid();
 
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
-      delete info;
-      sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
-      break;
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
+      if (profiler_is_active()) {
+        // We still want to show the results of this thread if you
+        // save the profile shortly after a thread is terminated.
+        // For now we will defer the delete to profile stop.
+        info->SetPendingDelete();
+        break;
+      } else {
+        delete info;
+        sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
+        break;
+      }
     }
   }
 
   uwt__unregister_thread_for_profiling();
 }
 
 #ifdef ANDROID
 static struct sigaction old_sigstart_signal_handler;
--- a/tools/profiler/platform-macos.cc
+++ b/tools/profiler/platform-macos.cc
@@ -207,17 +207,17 @@ class SamplerThread : public Thread {
         mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
         std::vector<ThreadInfo*> threads =
           SamplerRegistry::sampler->GetRegisteredThreads();
         bool isFirstProfiledThread = true;
         for (uint32_t i = 0; i < threads.size(); i++) {
           ThreadInfo* info = threads[i];
 
           // This will be null if we're not interested in profiling this thread.
-          if (!info->Profile())
+          if (!info->Profile() || info->IsPendingDelete())
             continue;
 
           PseudoStack::SleepState sleeping = info->Stack()->observeSleeping();
           if (sleeping == PseudoStack::SLEEPING_AGAIN) {
             info->Profile()->DuplicateLastSample();
             //XXX: This causes flushes regardless of jank-only mode
             info->Profile()->flush();
             continue;
@@ -360,17 +360,17 @@ bool Sampler::RegisterCurrentThread(cons
     return false;
 
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = gettid();
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
       // Thread already registered. This means the first unregister will be
       // too early.
       ASSERT(false);
       return false;
     }
   }
 
   set_tls_stack_top(stackTop);
@@ -396,20 +396,28 @@ void Sampler::UnregisterCurrentThread()
   tlsStackTop.set(nullptr);
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = gettid();
 
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
-      delete info;
-      sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
-      break;
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
+      if (profiler_is_active()) {
+        // We still want to show the results of this thread if you
+        // save the profile shortly after a thread is terminated.
+        // For now we will defer the delete to profile stop.
+        info->SetPendingDelete();
+        break;
+      } else {
+        delete info;
+        sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
+        break;
+      }
     }
   }
 }
 
 void TickSample::PopulateContext(void* aContext)
 {
   // Note that this asm changes if PopulateContext's parameter list is altered
 #if defined(SPS_PLAT_amd64_darwin)
--- a/tools/profiler/platform-win32.cc
+++ b/tools/profiler/platform-win32.cc
@@ -124,17 +124,17 @@ class SamplerThread : public Thread {
         mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
         std::vector<ThreadInfo*> threads =
           sampler_->GetRegisteredThreads();
         bool isFirstProfiledThread = true;
         for (uint32_t i = 0; i < threads.size(); i++) {
           ThreadInfo* info = threads[i];
 
           // This will be null if we're not interested in profiling this thread.
-          if (!info->Profile())
+          if (!info->Profile() || info->IsPendingDelete())
             continue;
 
           PseudoStack::SleepState sleeping = info->Stack()->observeSleeping();
           if (sleeping == PseudoStack::SLEEPING_AGAIN) {
             info->Profile()->DuplicateLastSample();
             //XXX: This causes flushes regardless of jank-only mode
             info->Profile()->flush();
             continue;
@@ -311,17 +311,17 @@ bool Sampler::RegisterCurrentThread(cons
 
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = GetCurrentThreadId();
 
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
       // Thread already registered. This means the first unregister will be
       // too early.
       ASSERT(false);
       return false;
     }
   }
 
   set_tls_stack_top(stackTop);
@@ -347,20 +347,28 @@ void Sampler::UnregisterCurrentThread()
   tlsStackTop.set(nullptr);
 
   mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
 
   int id = GetCurrentThreadId();
 
   for (uint32_t i = 0; i < sRegisteredThreads->size(); i++) {
     ThreadInfo* info = sRegisteredThreads->at(i);
-    if (info->ThreadId() == id) {
-      delete info;
-      sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
-      break;
+    if (info->ThreadId() == id && !info->IsPendingDelete()) {
+      if (profiler_is_active()) {
+        // We still want to show the results of this thread if you
+        // save the profile shortly after a thread is terminated.
+        // For now we will defer the delete to profile stop.
+        info->SetPendingDelete();
+        break;
+      } else {
+        delete info;
+        sRegisteredThreads->erase(sRegisteredThreads->begin() + i);
+        break;
+      }
     }
   }
 }
 
 void TickSample::PopulateContext(void* aContext)
 {
   MOZ_ASSERT(aContext);
   CONTEXT* pContext = reinterpret_cast<CONTEXT*>(aContext);
--- a/tools/profiler/platform.cpp
+++ b/tools/profiler/platform.cpp
@@ -81,16 +81,20 @@ static const char * gGeckoThreadName = "
 
 void Sampler::Startup() {
   sRegisteredThreads = new std::vector<ThreadInfo*>();
   sRegisteredThreadsMutex = new mozilla::Mutex("sRegisteredThreads mutex");
 }
 
 void Sampler::Shutdown() {
   while (sRegisteredThreads->size() > 0) {
+    // Any stack that's still referenced at this point are
+    // still active and we don't have a way to clean them up
+    // safetly and still handle the pop call on that object.
+    sRegisteredThreads->back()->ForgetStack();
     delete sRegisteredThreads->back();
     sRegisteredThreads->pop_back();
   }
 
   delete sRegisteredThreadsMutex;
   delete sRegisteredThreads;
 
   // UnregisterThread can be called after shutdown in XPCShell. Thus
@@ -104,27 +108,42 @@ ThreadInfo::ThreadInfo(const char* aName
                        void* aStackTop)
   : mName(strdup(aName))
   , mThreadId(aThreadId)
   , mIsMainThread(aIsMainThread)
   , mPseudoStack(aPseudoStack)
   , mPlatformData(Sampler::AllocPlatformData(aThreadId))
   , mProfile(nullptr)
   , mStackTop(aStackTop)
+  , mPendingDelete(false)
 {
   mThread = NS_GetCurrentThread();
 }
 
 ThreadInfo::~ThreadInfo() {
   free(mName);
 
   if (mProfile)
     delete mProfile;
 
   Sampler::FreePlatformData(mPlatformData);
+
+  delete mPseudoStack;
+  mPseudoStack = nullptr;
+}
+
+void
+ThreadInfo::SetPendingDelete()
+{
+  mPendingDelete = true;
+  // We don't own the pseudostack so disconnect it.
+  mPseudoStack = nullptr;
+  if (mProfile) {
+    mProfile->SetPendingDelete();
+  }
 }
 
 ProfilerMarker::ProfilerMarker(const char* aMarkerName,
     ProfilerMarkerPayload* aPayload,
     float aTime)
   : mMarkerName(strdup(aMarkerName))
   , mPayload(aPayload)
   , mTime(aTime)
@@ -716,16 +735,19 @@ void mozilla_sampler_start(int aProfileE
   tlsTicker.set(t);
   t->Start();
   if (t->ProfileJS() || t->InPrivacyMode()) {
       mozilla::MutexAutoLock lock(*Sampler::sRegisteredThreadsMutex);
       std::vector<ThreadInfo*> threads = t->GetRegisteredThreads();
 
       for (uint32_t i = 0; i < threads.size(); i++) {
         ThreadInfo* info = threads[i];
+        if (info->IsPendingDelete()) {
+          continue;
+        }
         ThreadProfile* thread_profile = info->Profile();
         if (!thread_profile) {
           continue;
         }
         thread_profile->GetPseudoStack()->reinitializeOnResume();
         if (t->ProfileJS()) {
           thread_profile->GetPseudoStack()->enableJSSampling();
         }
@@ -898,24 +920,23 @@ bool mozilla_sampler_register_thread(con
 }
 
 void mozilla_sampler_unregister_thread()
 {
   if (sInitCount == 0) {
     return;
   }
 
-  Sampler::UnregisterCurrentThread();
-
   PseudoStack *stack = tlsPseudoStack.get();
   if (!stack) {
     return;
   }
-  delete stack;
   tlsPseudoStack.set(nullptr);
+
+  Sampler::UnregisterCurrentThread();
 }
 
 void mozilla_sampler_sleep_start() {
     if (sInitCount == 0) {
 	return;
     }
 
     PseudoStack *stack = tlsPseudoStack.get();
--- a/tools/profiler/platform.h
+++ b/tools/profiler/platform.h
@@ -397,31 +397,41 @@ class ThreadInfo {
 
   virtual ~ThreadInfo();
 
   const char* Name() const { return mName; }
   int ThreadId() const { return mThreadId; }
 
   bool IsMainThread() const { return mIsMainThread; }
   PseudoStack* Stack() const { return mPseudoStack; }
-  
+  PseudoStack* ForgetStack() {
+    PseudoStack* stack = mPseudoStack;
+    mPseudoStack = nullptr;
+    return stack;
+  }
+
   void SetProfile(ThreadProfile* aProfile) { mProfile = aProfile; }
   ThreadProfile* Profile() const { return mProfile; }
 
   PlatformData* GetPlatformData() const { return mPlatformData; }
   void* StackTop() const { return mStackTop; }
 
+  void SetPendingDelete();
+  bool IsPendingDelete() const { return mPendingDelete; }
+
+
   /**
    * May be null for the main thread if the profiler was started during startup
    */
   nsIThread* GetThread() const { return mThread.get(); }
  private:
   char* mName;
   int mThreadId;
   const bool mIsMainThread;
   PseudoStack* mPseudoStack;
   PlatformData* mPlatformData;
   ThreadProfile* mProfile;
   void* const mStackTop;
   nsCOMPtr<nsIThread> mThread;
+  bool mPendingDelete;
 };
 
 #endif /* ndef TOOLS_PLATFORM_H_ */
--- a/tools/profiler/tests/gtest/ThreadProfileTest.cpp
+++ b/tools/profiler/tests/gtest/ThreadProfileTest.cpp
@@ -4,61 +4,61 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gtest/gtest.h"
 
 #include "ProfileEntry.h"
 
 // Make sure we can initialize our ThreadProfile
 TEST(ThreadProfile, Initialization) {
-  PseudoStack stack;
+  PseudoStack* stack = new PseudoStack();
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, &stack, nullptr);
+  ThreadInfo info("testThread", tid, true, stack, nullptr);
   ThreadProfile tp(&info, 10);
 }
 
 // Make sure we can record one tag and read it
 TEST(ThreadProfile, InsertOneTag) {
-  PseudoStack stack;
+  PseudoStack* stack = new PseudoStack();
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, &stack, nullptr);
+  ThreadInfo info("testThread", tid, true, stack, nullptr);
   ThreadProfile tp(&info, 10);
   tp.addTag(ProfileEntry('t', 123.1f));
   ASSERT_TRUE(tp.mEntries != nullptr);
   ASSERT_TRUE(tp.mEntries[tp.mReadPos].mTagName == 't');
   ASSERT_TRUE(tp.mEntries[tp.mReadPos].mTagFloat == 123.1f);
 }
 
 // See if we can insert some tags
 TEST(ThreadProfile, InsertTagsNoWrap) {
-  PseudoStack stack;
+  PseudoStack* stack = new PseudoStack();
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, &stack, nullptr);
+  ThreadInfo info("testThread", tid, true, stack, nullptr);
   ThreadProfile tp(&info, 100);
   int test_size = 50;
   for (int i = 0; i < test_size; i++) {
     tp.addTag(ProfileEntry('t', i));
   }
   ASSERT_TRUE(tp.mEntries != nullptr);
   int readPos = tp.mReadPos;
   while (readPos != tp.mWritePos) {
     ASSERT_TRUE(tp.mEntries[readPos].mTagName == 't');
     ASSERT_TRUE(tp.mEntries[readPos].mTagInt == readPos);
     readPos = (readPos + 1) % tp.mEntrySize;
   }
 }
 
 // See if wrapping works as it should in the basic case
 TEST(ThreadProfile, InsertTagsWrap) {
-  PseudoStack stack;
+  PseudoStack* stack = new PseudoStack();
   Thread::tid_t tid = 1000;
   // we can fit only 24 tags in this buffer because of the empty slot
   int tags = 24;
   int buffer_size = tags + 1;
-  ThreadInfo info("testThread", tid, true, &stack, nullptr);
+  ThreadInfo info("testThread", tid, true, stack, nullptr);
   ThreadProfile tp(&info, buffer_size);
   int test_size = 43;
   for (int i = 0; i < test_size; i++) {
     tp.addTag(ProfileEntry('t', i));
   }
   ASSERT_TRUE(tp.mEntries != nullptr);
   int readPos = tp.mReadPos;
   int ctr = 0;