Bug 1340928 (part 1) - Two small platform-linux-android.cpp tweaks. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 15 Feb 2017 14:44:07 +1100
changeset 373440 5b86ca9a7e8e3364e64fbab7c777f89305e0592e
parent 373439 7771cc7440c7171c59f2c330cba3d3428bfc2032
child 373441 a3a18d2124d73caac76f56fc87410fcaec7336b4
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 1) - Two small platform-linux-android.cpp tweaks. r=mstange. - Don't bother checking gSampler in ProfilerSignalHandler. It is equivalent to checking gIsActive and we do that at the top of the loop in SignalSender(). There is no point repeatedly checking the same condition in the middle of that loop; that just opens up the possibility of partially complete samples where some threads are missing. - Clear gCurrentThreadInfo in SignalSender() instead of in ProfilerSignalHandler(). The effect is much the same, but this change means gCurrentThreadInfo is both set and cleared in SignalSender(), i.e. on a single thread, removing any need for Atomic<>.
tools/profiler/core/platform-linux-android.cpp
--- a/tools/profiler/core/platform-linux-android.cpp
+++ b/tools/profiler/core/platform-linux-android.cpp
@@ -57,17 +57,16 @@
 #ifdef __GLIBC__
 #include <execinfo.h>   // backtrace, backtrace_symbols
 #endif  // def __GLIBC__
 #include <strings.h>    // index
 #include <errno.h>
 #include <stdarg.h>
 
 #include "prenv.h"
-#include "mozilla/Atomics.h"
 #include "mozilla/LinuxSignal.h"
 #include "mozilla/DebugOnly.h"
 #include "ProfileEntry.h"
 
 // Memory profile
 #include "nsMemoryReporterManager.h"
 
 #include <string.h>
@@ -141,17 +140,17 @@ 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 mozilla::Atomic<ThreadInfo*> gCurrentThreadInfo;
+static ThreadInfo* gCurrentThreadInfo;
 static sem_t gSignalHandlingDone;
 
 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;
 #if defined(GP_ARCH_x86)
@@ -173,37 +172,29 @@ static void SetSampleContext(TickSample*
 }
 
 static void
 SigprofHandler(int signal, siginfo_t* info, void* context)
 {
   // Avoid TSan warning about clobbering errno.
   int savedErrno = errno;
 
-  // XXX: this is an off-main-thread(?) use of gSampler
-  if (!gSampler) {
-    sem_post(&gSignalHandlingDone);
-    errno = savedErrno;
-    return;
-  }
-
   TickSample sample_obj;
   TickSample* sample = &sample_obj;
   sample->context = context;
 
   // Extract the current pc and sp.
   SetSampleContext(sample, context);
   sample->threadInfo = gCurrentThreadInfo;
   sample->timestamp = mozilla::TimeStamp::Now();
   sample->rssMemory = sample->threadInfo->mRssMemory;
   sample->ussMemory = sample->threadInfo->mUssMemory;
 
   Tick(sample);
 
-  gCurrentThreadInfo = NULL;
   sem_post(&gSignalHandlingDone);
   errno = savedErrno;
 }
 
 #if defined(GP_OS_android)
 #define SYS_tgkill __NR_tgkill
 #endif
 
@@ -321,18 +312,20 @@ SigprofSender(void* aArg)
           printf_stderr("profiler failed to signal tid=%d\n", threadId);
 #ifdef DEBUG
           abort();
 #else
           continue;
 #endif
         }
 
-        // Wait for the signal handler to run before moving on to the next one
+        // Wait for the signal handler to run before moving on to the next one.
         sem_wait(&gSignalHandlingDone);
+
+        gCurrentThreadInfo = nullptr;
         isFirstProfiledThread = false;
       }
 #if defined(USE_LUL_STACKWALK)
       // The LUL unwind object accumulates frame statistics. Periodically we
       // 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.