Bug 1336326 (part 3) - Remove SamplerRegistry. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 03 Feb 2017 13:18:02 +1100
changeset 479271 65c5923d0feccdbe87b3dfc50a389e715d775a70
parent 479270 32550b42266999359049ab671fce22fdb5dc4f7b
child 479303 1cc159c7a0445ec51e335c8a1a1cceea7bbf8380
child 479367 74d2f566ed65b8ba65db684a24279bb9ad984f8b
push id44211
push userbmo:hskupin@gmail.com
push dateMon, 06 Feb 2017 12:18:11 +0000
reviewersmstange
bugs1336326
milestone54.0a1
Bug 1336326 (part 3) - Remove SamplerRegistry. r=mstange. We only have one global Sampler, gSampler, and so SamplerRegistry is just an unnecessary and obfuscating wrapper around it.
tools/profiler/core/platform-linux.cc
tools/profiler/core/platform-macos.cc
tools/profiler/core/platform-win32.cc
--- a/tools/profiler/core/platform-linux.cc
+++ b/tools/profiler/core/platform-linux.cc
@@ -156,29 +156,16 @@ static void paf_parent(void) {
 
 // Set up the fork handlers.
 static void* setup_atfork() {
   pthread_atfork(paf_prepare, paf_parent, NULL);
   return NULL;
 }
 #endif /* !defined(ANDROID) */
 
-struct SamplerRegistry {
-  static void AddActiveSampler(Sampler *sampler) {
-    MOZ_ASSERT(!SamplerRegistry::sampler);
-    SamplerRegistry::sampler = sampler;
-  }
-  static void RemoveActiveSampler(Sampler *sampler) {
-    SamplerRegistry::sampler = NULL;
-  }
-  static Sampler *sampler;
-};
-
-Sampler *SamplerRegistry::sampler = NULL;
-
 static mozilla::Atomic<ThreadInfo*> sCurrentThreadInfo;
 static sem_t sSignalHandlingDone;
 
 static void SetSampleContext(TickSample* sample, void* context)
 {
   // Extracting the sample from the context is extremely machine dependent.
   ucontext_t* ucontext = reinterpret_cast<ucontext_t*>(context);
   mcontext_t& mcontext = ucontext->uc_mcontext;
@@ -299,21 +286,21 @@ static void* SignalSender(void* arg) {
 
   int vm_tgid_ = getpid();
   DebugOnly<int> my_tid = gettid();
 
   unsigned int nSignalsSent = 0;
 
   TimeDuration lastSleepOverhead = 0;
   TimeStamp sampleStart = TimeStamp::Now();
-  while (SamplerRegistry::sampler->IsActive()) {
+  // XXX: this loop is an off-main-thread use of gSampler
+  while (gSampler->IsActive()) {
+    gSampler->DeleteExpiredMarkers();
 
-    SamplerRegistry::sampler->DeleteExpiredMarkers();
-
-    if (!SamplerRegistry::sampler->IsPaused()) {
+    if (!gSampler->IsPaused()) {
       StaticMutexAutoLock lock(Sampler::sRegisteredThreadsMutex);
 
       bool isFirstProfiledThread = true;
       for (uint32_t i = 0; i < Sampler::sRegisteredThreads->size(); i++) {
         ThreadInfo* info = (*Sampler::sRegisteredThreads)[i];
 
         // This will be null if we're not interested in profiling this thread.
         if (!info->hasProfile() || info->IsPendingDelete()) {
@@ -364,17 +351,18 @@ static void* SignalSender(void* arg) {
         if ((++nSignalsSent & 0xF) == 0) {
 #          if defined(USE_LUL_STACKWALK)
            sLUL->MaybeShowStats();
 #          endif
         }
       }
     }
 
-    TimeStamp targetSleepEndTime = sampleStart + TimeDuration::FromMicroseconds(SamplerRegistry::sampler->interval() * 1000);
+    TimeStamp targetSleepEndTime =
+      sampleStart + TimeDuration::FromMicroseconds(gSampler->interval() * 1000);
     TimeStamp beforeSleep = TimeStamp::Now();
     TimeDuration targetSleepDuration = targetSleepEndTime - beforeSleep;
     double sleepTime = std::max(0.0, (targetSleepDuration - lastSleepOverhead).ToMicroseconds());
     OS::SleepMicro(sleepTime);
     sampleStart = TimeStamp::Now();
     lastSleepOverhead = sampleStart - (beforeSleep + TimeDuration::FromMicroseconds(sleepTime));
   }
   return 0;
@@ -388,18 +376,16 @@ void Sampler::Start() {
 #elif defined(USE_LUL_STACKWALK)
   // NOTE: this isn't thread-safe.  But we expect Sampler::Start to be
   // called only from the main thread, so this is OK in general.
   if (!sLUL) {
      sLUL_initialization_routine();
   }
 #endif
 
-  SamplerRegistry::AddActiveSampler(this);
-
   // Initialize signal handler communication
   sCurrentThreadInfo = nullptr;
   if (sem_init(&sSignalHandlingDone, /* pshared: */ 0, /* value: */ 0) != 0) {
     LOG("Error initializing semaphore");
     return;
   }
 
   // Request profiling signals.
@@ -445,18 +431,16 @@ void Sampler::Stop() {
 
   // Wait for signal sender termination (it will exit after setting
   // active_ to false).
   if (signal_sender_launched_) {
     pthread_join(signal_sender_thread_, NULL);
     signal_sender_launched_ = false;
   }
 
-  SamplerRegistry::RemoveActiveSampler(this);
-
   // Restore old signal handler
   if (signal_handler_installed_) {
     sigaction(SIGPROF, &old_sigprof_signal_handler_, 0);
     signal_handler_installed_ = false;
   }
 }
 
 #ifdef ANDROID
--- a/tools/profiler/core/platform-macos.cc
+++ b/tools/profiler/core/platform-macos.cc
@@ -38,31 +38,16 @@
 #include "platform.h"
 #include "mozilla/TimeStamp.h"
 
 using mozilla::TimeStamp;
 using mozilla::TimeDuration;
 
 // this port is based off of v8 svn revision 9837
 
-// XXX: this is a very stubbed out implementation
-// that only supports a single Sampler
-struct SamplerRegistry {
-  static void AddActiveSampler(Sampler *sampler) {
-    MOZ_ASSERT(!SamplerRegistry::sampler);
-    SamplerRegistry::sampler = sampler;
-  }
-  static void RemoveActiveSampler(Sampler *sampler) {
-    SamplerRegistry::sampler = NULL;
-  }
-  static Sampler *sampler;
-};
-
-Sampler *SamplerRegistry::sampler = NULL;
-
 #ifdef DEBUG
 // 0 is never a valid thread id on MacOSX since a pthread_t is a pointer.
 static const pthread_t kNoThread = (pthread_t) 0;
 #endif
 
 void OS::Startup() {
 }
 
@@ -148,40 +133,40 @@ public:
     pthread_create(&mThread, attr_ptr, ThreadEntry, this);
     MOZ_ASSERT(mThread != kNoThread);
   }
 
   void Join() {
     pthread_join(mThread, NULL);
   }
 
-  static void AddActiveSampler(Sampler* sampler) {
-    SamplerRegistry::AddActiveSampler(sampler);
+  static void AddActiveSampler() {
+    MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
     if (mInstance == NULL) {
-      mInstance = new SamplerThread(sampler->interval());
+      mInstance = new SamplerThread(gSampler->interval());
       mInstance->Start();
     }
   }
 
-  static void RemoveActiveSampler(Sampler* sampler) {
+  static void RemoveActiveSampler() {
     mInstance->Join();
-    //XXX: unlike v8 we need to remove the active sampler after doing the Join
-    // because we drop the sampler immediately
-    SamplerRegistry::RemoveActiveSampler(sampler);
     delete mInstance;
     mInstance = NULL;
   }
 
   void Run() {
     TimeDuration lastSleepOverhead = 0;
     TimeStamp sampleStart = TimeStamp::Now();
-    while (SamplerRegistry::sampler->IsActive()) {
-      SamplerRegistry::sampler->DeleteExpiredMarkers();
 
-      if (!SamplerRegistry::sampler->IsPaused()) {
+    // XXX: this loop is an off-main-thread use of gSampler
+    while (gSampler->IsActive()) {
+      gSampler->DeleteExpiredMarkers();
+
+      if (!gSampler->IsPaused()) {
         StaticMutexAutoLock lock(Sampler::sRegisteredThreadsMutex);
 
         bool isFirstProfiledThread = true;
         for (uint32_t i = 0; i < Sampler::sRegisteredThreads->size(); i++) {
           ThreadInfo* info = (*Sampler::sRegisteredThreads)[i];
 
           // This will be null if we're not interested in profiling this thread.
           if (!info->hasProfile() || info->IsPendingDelete()) {
@@ -191,33 +176,32 @@ public:
           PseudoStack::SleepState sleeping = info->Stack()->observeSleeping();
           if (sleeping == PseudoStack::SLEEPING_AGAIN) {
             info->DuplicateLastSample();
             continue;
           }
 
           info->UpdateThreadResponsiveness();
 
-          SampleContext(SamplerRegistry::sampler, info, isFirstProfiledThread);
+          SampleContext(info, isFirstProfiledThread);
           isFirstProfiledThread = false;
         }
       }
 
       TimeStamp targetSleepEndTime = sampleStart + TimeDuration::FromMicroseconds(mIntervalMicro);
       TimeStamp beforeSleep = TimeStamp::Now();
       TimeDuration targetSleepDuration = targetSleepEndTime - beforeSleep;
       double sleepTime = std::max(0.0, (targetSleepDuration - lastSleepOverhead).ToMicroseconds());
       OS::SleepMicro(sleepTime);
       sampleStart = TimeStamp::Now();
       lastSleepOverhead = sampleStart - (beforeSleep + TimeDuration::FromMicroseconds(sleepTime));
     }
   }
 
-  void SampleContext(Sampler* sampler, ThreadInfo* aThreadInfo,
-                     bool isFirstProfiledThread)
+  void SampleContext(ThreadInfo* aThreadInfo, bool isFirstProfiledThread)
   {
     thread_act_t profiled_thread =
       aThreadInfo->GetPlatformData()->profiled_thread();
 
     TickSample sample_obj;
     TickSample* sample = &sample_obj;
 
     // Unique Set Size is not supported on Mac.
@@ -262,17 +246,18 @@ public:
                          reinterpret_cast<natural_t*>(&state),
                          &count) == KERN_SUCCESS) {
       sample->pc = reinterpret_cast<Address>(state.REGISTER_FIELD(ip));
       sample->sp = reinterpret_cast<Address>(state.REGISTER_FIELD(sp));
       sample->fp = reinterpret_cast<Address>(state.REGISTER_FIELD(bp));
       sample->timestamp = mozilla::TimeStamp::Now();
       sample->threadInfo = aThreadInfo;
 
-      sampler->Tick(sample);
+      // XXX: this is an off-main-thread use of gSampler
+      gSampler->Tick(sample);
     }
     thread_resume(profiled_thread);
   }
 
 private:
   pthread_t mThread;
 
   int mIntervalMicro;
@@ -284,23 +269,23 @@ private:
 
 #undef REGISTER_FIELD
 
 SamplerThread* SamplerThread::mInstance = NULL;
 
 void Sampler::Start() {
   MOZ_ASSERT(!IsActive());
   SetActive(true);
-  SamplerThread::AddActiveSampler(this);
+  SamplerThread::AddActiveSampler();
 }
 
 void Sampler::Stop() {
   MOZ_ASSERT(IsActive());
   SetActive(false);
-  SamplerThread::RemoveActiveSampler(this);
+  SamplerThread::RemoveActiveSampler();
 }
 
 /* static */ Thread::tid_t
 Thread::GetCurrentId()
 {
   return gettid();
 }
 
--- a/tools/profiler/core/platform-win32.cc
+++ b/tools/profiler/core/platform-win32.cc
@@ -91,20 +91,19 @@ static const HANDLE kNoThread = INVALID_
 // Start() method is called the new thread starts running the Run() method in
 // the new thread. The SamplerThread object should not be deallocated before
 // the thread has terminated.
 class SamplerThread
 {
  public:
   // Initialize a Win32 thread object. The thread has an invalid thread
   // handle until it is started.
-  SamplerThread(double interval, Sampler* sampler)
+  explicit SamplerThread(double interval)
     : mStackSize(0)
     , mThread(kNoThread)
-    , mSampler(sampler)
     , mInterval(interval)
   {
     mInterval = floor(interval + 0.5);
     if (mInterval <= 0) {
       mInterval = 1;
     }
   }
 
@@ -135,42 +134,45 @@ class SamplerThread
   }
 
   void Join() {
     if (mThreadId != Thread::GetCurrentId()) {
       WaitForSingleObject(mThread, INFINITE);
     }
   }
 
-  static void StartSampler(Sampler* sampler) {
+  static void StartSampler() {
+    MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
     if (mInstance == NULL) {
-      mInstance = new SamplerThread(sampler->interval(), sampler);
+      mInstance = new SamplerThread(gSampler->interval());
       mInstance->Start();
     } else {
-      MOZ_ASSERT(mInstance->mInterval == sampler->interval());
+      MOZ_ASSERT(mInstance->mInterval == gSampler->interval());
     }
   }
 
   static void StopSampler() {
     mInstance->Join();
     delete mInstance;
     mInstance = NULL;
   }
 
   void Run() {
     // By default we'll not adjust the timer resolution which tends to be around
     // 16ms. However, if the requested interval is sufficiently low we'll try to
     // adjust the resolution to match.
     if (mInterval < 10)
         ::timeBeginPeriod(mInterval);
 
-    while (mSampler->IsActive()) {
-      mSampler->DeleteExpiredMarkers();
+    // XXX: this loop is an off-main-thread use of gSampler
+    while (gSampler->IsActive()) {
+      gSampler->DeleteExpiredMarkers();
 
-      if (!mSampler->IsPaused()) {
+      if (!gSampler->IsPaused()) {
         mozilla::StaticMutexAutoLock lock(Sampler::sRegisteredThreadsMutex);
 
         bool isFirstProfiledThread = true;
         for (uint32_t i = 0; i < Sampler::sRegisteredThreads->size(); i++) {
           ThreadInfo* info = (*Sampler::sRegisteredThreads)[i];
 
           // This will be null if we're not interested in profiling this thread.
           if (!info->hasProfile() || info->IsPendingDelete()) {
@@ -180,30 +182,29 @@ class SamplerThread
           PseudoStack::SleepState sleeping = info->Stack()->observeSleeping();
           if (sleeping == PseudoStack::SLEEPING_AGAIN) {
             info->DuplicateLastSample();
             continue;
           }
 
           info->UpdateThreadResponsiveness();
 
-          SampleContext(mSampler, info, isFirstProfiledThread);
+          SampleContext(info, isFirstProfiledThread);
           isFirstProfiledThread = false;
         }
       }
       OS::Sleep(mInterval);
     }
 
     // disable any timer resolution changes we've made
     if (mInterval < 10)
         ::timeEndPeriod(mInterval);
   }
 
-  void SampleContext(Sampler* sampler, ThreadInfo* aThreadInfo,
-                     bool isFirstProfiledThread)
+  void SampleContext(ThreadInfo* aThreadInfo, bool isFirstProfiledThread)
   {
     uintptr_t thread = Sampler::GetThreadHandle(aThreadInfo->GetPlatformData());
     HANDLE profiled_thread = reinterpret_cast<HANDLE>(thread);
     if (profiled_thread == NULL)
       return;
 
     // Context used for sampling the register state of the profiled thread.
     CONTEXT context;
@@ -275,41 +276,42 @@ class SamplerThread
     sample->fp = reinterpret_cast<Address>(context.Rbp);
 #else
     sample->pc = reinterpret_cast<Address>(context.Eip);
     sample->sp = reinterpret_cast<Address>(context.Esp);
     sample->fp = reinterpret_cast<Address>(context.Ebp);
 #endif
 
     sample->context = &context;
-    sampler->Tick(sample);
+
+    // XXX: this is an off-main-thread use of gSampler
+    gSampler->Tick(sample);
 
     ResumeThread(profiled_thread);
   }
 
 private:
   int mStackSize;
   HANDLE mThread;
   Thread::tid_t mThreadId;
 
-  Sampler* mSampler;
   int mInterval; // units: ms
 
   // Protects the process wide state below.
   static SamplerThread* mInstance;
 
   DISALLOW_COPY_AND_ASSIGN(SamplerThread);
 };
 
 SamplerThread* SamplerThread::mInstance = NULL;
 
 void Sampler::Start() {
   MOZ_ASSERT(!IsActive());
   SetActive(true);
-  SamplerThread::StartSampler(this);
+  SamplerThread::StartSampler();
 }
 
 void Sampler::Stop() {
   MOZ_ASSERT(IsActive());
   SetActive(false);
   SamplerThread::StopSampler();
 }