Bug 1340928 (part 5) - Pass the interval to PlatformStart(). r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 15 Feb 2017 14:44:12 +1100
changeset 373444 7e6850af9372989b7dd66318076deb6df02ddf55
parent 373443 c043ce740d1bed8c359290b5b7125cc27f238147
child 373445 8ac1a3617dc977f6d4f36e644f41a1dd5acad6ff
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1340928
milestone54.0a1
Bug 1340928 (part 5) - Pass the interval to PlatformStart(). r=mstange. This avoids the need for platform-linux-android.cpp to read gInterval off the main thread in an awkward spot. It also makes platform-linux-android.cpp more like platform-{win32,macos}.cpp.
tools/profiler/core/platform-linux-android.cpp
tools/profiler/core/platform-macos.cpp
tools/profiler/core/platform-win32.cpp
tools/profiler/core/platform.cpp
--- a/tools/profiler/core/platform-linux-android.cpp
+++ b/tools/profiler/core/platform-linux-android.cpp
@@ -140,16 +140,18 @@ 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(GP_OS_android) */
 
+static int gIntervalMicro;
+
 // Global variables through which data is sent from SigprofSender() to
 // SigprofHandler(). gSignalHandlingDone provides inter-thread synchronization.
 static ThreadInfo* gCurrentThreadInfo;
 static int64_t gRssMemory;
 static int64_t gUssMemory;
 
 // Semaphore used to coordinate SigprofSender() and SigprofHandler().
 static sem_t gSignalHandlingDone;
@@ -337,46 +339,48 @@ SigprofSender(void* aArg)
       // should poke it to give it a chance to print those statistics. This
       // involves doing I/O (fprintf, __android_log_print, etc.) and so can't
       // safely be done from the unwinder threads, which is why it is done
       // here.
       gLUL->MaybeShowStats();
 #endif
     }
 
-    // This off-main-thread use of gInterval is safe due to implicit
-    // synchronization -- this function cannot run at the same time as
-    // profiler_{start,stop}(), where gInterval is set.
     TimeStamp targetSleepEndTime =
-      sampleStart + TimeDuration::FromMicroseconds(gInterval * 1000);
+      sampleStart + TimeDuration::FromMicroseconds(gIntervalMicro);
     TimeStamp beforeSleep = TimeStamp::Now();
     TimeDuration targetSleepDuration = targetSleepEndTime - beforeSleep;
     double sleepTime = std::max(0.0, (targetSleepDuration - lastSleepOverhead).ToMicroseconds());
     SleepMicro(sleepTime);
     sampleStart = TimeStamp::Now();
     lastSleepOverhead = sampleStart - (beforeSleep + TimeDuration::FromMicroseconds(sleepTime));
   }
   return 0;
 }
 
 static void
-PlatformStart()
+PlatformStart(double aInterval)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
 #if defined(USE_EHABI_STACKWALK)
   mozilla::EHABIStackWalkInit();
 #elif defined(USE_LUL_STACKWALK)
   // NOTE: this isn't thread-safe.  But we expect PlatformStart() to be
   // called only from the main thread, so this is OK in general.
   if (!gLUL) {
      gLUL_initialization_routine();
   }
 #endif
 
+  gIntervalMicro = floor(aInterval * 1000 + 0.5);
+  if (gIntervalMicro <= 0) {
+    gIntervalMicro = 1;
+  }
+
   // Initialize signal handler communication
   gCurrentThreadInfo = nullptr;
   gRssMemory = 0;
   gUssMemory = 0;
   if (sem_init(&gSignalHandlingDone, /* pshared: */ 0, /* value: */ 0) != 0) {
     LOG("Error initializing semaphore");
     return;
   }
@@ -421,16 +425,18 @@ PlatformStart()
 static void
 PlatformStop()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(gIsActive);
   gIsActive = false;
 
+  gIntervalMicro = 0;
+
   // Wait for signal sender termination (it will exit after setting
   // active_ to false).
   if (gHasSigprofSenderLaunched) {
     pthread_join(gSigprofSenderThread, NULL);
     gHasSigprofSenderLaunched = false;
   }
 
   // Restore old signal handler
--- a/tools/profiler/core/platform-macos.cpp
+++ b/tools/profiler/core/platform-macos.cpp
@@ -75,18 +75,18 @@ PlatformDataDestructor::operator()(Platf
 }
 
 // The sampler thread controls sampling and runs whenever the profiler is
 // active. It periodically runs through all registered threads, finds those
 // that should be sampled, then pauses and samples them.
 class SamplerThread
 {
 public:
-  explicit SamplerThread(double interval)
-    : mIntervalMicro(floor(interval * 1000 + 0.5))
+  explicit SamplerThread(double aInterval)
+    : mIntervalMicro(floor(aInterval * 1000 + 0.5))
   {
     if (mIntervalMicro <= 0) {
       mIntervalMicro = 1;
     }
   }
 
   static void SetThreadName() {
     // pthread_setname_np is only available in 10.6 or later, so test
@@ -115,22 +115,22 @@ public:
     pthread_create(&mThread, attr_ptr, ThreadEntry, this);
     MOZ_ASSERT(mThread != kNoThread);
   }
 
   void Join() {
     pthread_join(mThread, NULL);
   }
 
-  static void StartSampler() {
+  static void StartSampler(double aInterval) {
     MOZ_RELEASE_ASSERT(NS_IsMainThread());
     MOZ_RELEASE_ASSERT(!mInstance);
 
     if (mInstance == NULL) {
-      mInstance = new SamplerThread(gInterval);
+      mInstance = new SamplerThread(aInterval);
       mInstance->Start();
     }
   }
 
   static void StopSampler() {
     MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
     mInstance->Join();
@@ -256,23 +256,23 @@ private:
 SamplerThread* SamplerThread::mInstance = NULL;
 
 static void
 PlatformInit()
 {
 }
 
 static void
-PlatformStart()
+PlatformStart(double aInterval)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(!gIsActive);
   gIsActive = true;
-  SamplerThread::StartSampler();
+  SamplerThread::StartSampler(aInterval);
 }
 
 static void
 PlatformStop()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(gIsActive);
--- a/tools/profiler/core/platform-win32.cpp
+++ b/tools/profiler/core/platform-win32.cpp
@@ -82,21 +82,20 @@ GetThreadHandle(PlatformData* aData)
 static const HANDLE kNoThread = INVALID_HANDLE_VALUE;
 
 // The sampler thread controls sampling and runs whenever the profiler is
 // active. It periodically runs through all registered threads, finds those
 // that should be sampled, then pauses and samples them.
 class SamplerThread
 {
  public:
-  explicit SamplerThread(double interval)
+  explicit SamplerThread(double aInterval)
     : mThread(kNoThread)
-    , mInterval(interval)
+    , mInterval(floor(aInterval + 0.5))
   {
-    mInterval = floor(interval + 0.5);
     if (mInterval <= 0) {
       mInterval = 1;
     }
   }
 
   ~SamplerThread() {
     // Close our own handle for the thread.
     if (mThread != kNoThread) {
@@ -124,21 +123,21 @@ class SamplerThread
   }
 
   void Join() {
     if (mThreadId != Thread::GetCurrentId()) {
       WaitForSingleObject(mThread, INFINITE);
     }
   }
 
-  static void StartSampler() {
+  static void StartSampler(double aInterval) {
     MOZ_RELEASE_ASSERT(NS_IsMainThread());
     MOZ_RELEASE_ASSERT(!mInstance);
 
-    mInstance = new SamplerThread(gInterval);
+    mInstance = new SamplerThread(aInterval);
     mInstance->Start();
   }
 
   static void StopSampler() {
     MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
     mInstance->Join();
     delete mInstance;
@@ -268,23 +267,23 @@ private:
 SamplerThread* SamplerThread::mInstance = NULL;
 
 static void
 PlatformInit()
 {
 }
 
 static void
-PlatformStart()
+PlatformStart(double aInterval)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(!gIsActive);
   gIsActive = true;
-  SamplerThread::StartSampler();
+  SamplerThread::StartSampler(aInterval);
 }
 
 static void
 PlatformStop()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(gIsActive);
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -117,21 +117,17 @@ static Vector<std::string> gThreadNameFi
 static mozilla::StaticMutex gThreadNameFiltersMutex;
 
 // All accesses to gFeatures are on the main thread, so no locking is needed.
 static Vector<std::string> gFeatures;
 
 // All accesses to gEntrySize are on the main thread, so no locking is needed.
 static int gEntrySize = 0;
 
-// This variable is set on the main thread in profiler_{start,stop}(), and
-// mostly read on the main thread. There is one read off the main thread in
-// SigprofSender() in platform-linux.cc which is safe because there is implicit
-// synchronization between that function and the set points in
-// profiler_{start,stop}().
+// All accesses to gInterval are on the main thread, so no locking is needed.
 static double gInterval = 0;
 
 // XXX: These two variables are used extensively both on and off the main
 // thread. It's possible that code that checks them then unsafely assumes their
 // values don't subsequently change.
 static Atomic<bool> gIsActive(false);
 static Atomic<bool> gIsPaused(false);
 
@@ -1586,17 +1582,17 @@ RegisterCurrentThread(const char* aName,
 
   MaybeSetProfile(info);
 
   gRegisteredThreads->push_back(info);
 }
 
 // Platform-specific init/start/stop actions.
 static void PlatformInit();
-static void PlatformStart();
+static void PlatformStart(double aInterval);
 static void PlatformStop();
 
 void
 profiler_init(void* stackTop)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   gInitCount++;
@@ -2027,17 +2023,17 @@ profiler_start(int aProfileEntries, doub
   if (gTaskTracer) {
     mozilla::tasktracer::StartLogging();
   }
 #endif
 
   gGatherer = new mozilla::ProfileGatherer();
 
   MOZ_ASSERT(!gIsActive && !gIsPaused);
-  PlatformStart();
+  PlatformStart(gInterval);
   MOZ_ASSERT(gIsActive && !gIsPaused);  // PlatformStart() sets gIsActive.
 
   if (gProfileJS || privacyMode) {
     mozilla::StaticMutexAutoLock lock(gRegisteredThreadsMutex);
 
     for (uint32_t i = 0; i < gRegisteredThreads->size(); i++) {
       ThreadInfo* info = (*gRegisteredThreads)[i];
       if (info->IsPendingDelete() || !info->hasProfile()) {