Bug 1336326 (part 3) - Remove SamplerRegistry. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 03 Feb 2017 13:18:02 +1100
changeset 340896 65c5923d0feccdbe87b3dfc50a389e715d775a70
parent 340895 32550b42266999359049ab671fce22fdb5dc4f7b
child 340897 1cc159c7a0445ec51e335c8a1a1cceea7bbf8380
child 340965 74d2f566ed65b8ba65db684a24279bb9ad984f8b
push id31318
push usercbook@mozilla.com
push dateMon, 06 Feb 2017 11:56:59 +0000
treeherdermozilla-central@1cc159c7a044 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1336326
milestone54.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 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();
 }