Bug 1339327 (part 7) - Rename some things in platform-linux.cc. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 15 Feb 2017 14:26:23 +1100
changeset 342949 542fffc2c125254f39a8457abfb69671269c6dd1
parent 342948 d6c8118d95f32d06261d69317e3d14f1b4fd9648
child 342950 e538c3740be131f78a98129221f16d4dacb29906
push id31366
push usercbook@mozilla.com
push dateWed, 15 Feb 2017 11:25:19 +0000
treeherdermozilla-central@c0807d6938c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1339327
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 1339327 (part 7) - Rename some things in platform-linux.cc. r=mstange. The new names are more consistent, and I find them clearer. The patch also inlines and removes the horribly-named ProfilerSignalThread() function.
tools/profiler/core/platform-linux.cc
tools/profiler/core/platform.cpp
tools/profiler/public/PseudoStack.h
--- a/tools/profiler/core/platform-linux.cc
+++ b/tools/profiler/core/platform-linux.cc
@@ -77,23 +77,23 @@
 
 #include <string.h>
 #include <list>
 
 using namespace mozilla;
 
 // All accesses to these two variables are on the main thread, so no locking is
 // needed.
-static bool gIsSigprofSignalHandlerInstalled;
-static struct sigaction gOldSigprofSignalHandler;
+static bool gIsSigprofHandlerInstalled;
+static struct sigaction gOldSigprofHandler;
 
 // All accesses to these two variables are on the main thread, so no locking is
 // needed.
-static bool gHasSignalSenderLaunched;
-static pthread_t gSignalSenderThread;
+static bool gHasSigprofSenderLaunched;
+static pthread_t gSigprofSenderThread;
 
 #if defined(USE_LUL_STACKWALK)
 // A singleton instance of the library.  It is initialised at first
 // use.  Currently only the main thread can call PlatformStart(), so
 // there is no need for a mechanism to ensure that it is only
 // created once in a multi-thread-use situation.
 lul::LUL* gLUL = nullptr;
 
@@ -189,19 +189,19 @@ static void SetSampleContext(TickSample*
 #ifdef ANDROID
 #define V8_HOST_ARCH_ARM 1
 #define SYS_gettid __NR_gettid
 #define SYS_tgkill __NR_tgkill
 #else
 #define V8_HOST_ARCH_X64 1
 #endif
 
-namespace {
-
-void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) {
+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;
@@ -220,30 +220,16 @@ void ProfilerSignalHandler(int signal, s
 
   Tick(sample);
 
   gCurrentThreadInfo = NULL;
   sem_post(&gSignalHandlingDone);
   errno = savedErrno;
 }
 
-} // namespace
-
-static void
-ProfilerSignalThread(ThreadInfo* aInfo, bool aIsFirstProfiledThread)
-{
-  if (aIsFirstProfiledThread && gProfileMemory) {
-    aInfo->mRssMemory = nsMemoryReporterManager::ResidentFast();
-    aInfo->mUssMemory = nsMemoryReporterManager::ResidentUnique();
-  } else {
-    aInfo->mRssMemory = 0;
-    aInfo->mUssMemory = 0;
-  }
-}
-
 int tgkill(pid_t tgid, pid_t tid, int signalno) {
   return syscall(SYS_tgkill, tgid, tid, signalno);
 }
 
 class PlatformData {
  public:
   PlatformData()
   {
@@ -263,17 +249,19 @@ AllocPlatformData(int aThreadId)
 }
 
 void
 PlatformDataDestructor::operator()(PlatformData* aData)
 {
   delete aData;
 }
 
-static void* SignalSender(void* arg) {
+static void*
+SigprofSender(void* aArg)
+{
   // This function runs on its own thread.
 
   // Taken from platform_thread_posix.cc
   prctl(PR_SET_NAME, "SamplerThread", 0, 0, 0);
 
   int vm_tgid_ = getpid();
   DebugOnly<int> my_tid = gettid();
 
@@ -308,17 +296,23 @@ static void* SignalSender(void* arg) {
         gCurrentThreadInfo = info;
 
         int threadId = info->ThreadId();
         MOZ_ASSERT(threadId != my_tid);
 
         // Profile from the signal sender for information which is not signal
         // safe, and will have low variation between the emission of the signal
         // and the signal handler catch.
-        ProfilerSignalThread(gCurrentThreadInfo, isFirstProfiledThread);
+        if (isFirstProfiledThread && gProfileMemory) {
+          info->mRssMemory = nsMemoryReporterManager::ResidentFast();
+          info->mUssMemory = nsMemoryReporterManager::ResidentUnique();
+        } else {
+          info->mRssMemory = 0;
+          info->mUssMemory = 0;
+        }
 
         // Profile from the signal handler for information which is signal safe
         // and needs to be precise too, such as the stack of the interrupted
         // thread.
         if (tgkill(vm_tgid_, threadId, SIGPROF) != 0) {
           printf_stderr("profiler failed to signal tid=%d\n", threadId);
 #ifdef DEBUG
           abort();
@@ -379,25 +373,25 @@ PlatformStart()
   if (sem_init(&gSignalHandlingDone, /* pshared: */ 0, /* value: */ 0) != 0) {
     LOG("Error initializing semaphore");
     return;
   }
 
   // Request profiling signals.
   LOG("Request signal");
   struct sigaction sa;
-  sa.sa_sigaction = MOZ_SIGNAL_TRAMPOLINE(ProfilerSignalHandler);
+  sa.sa_sigaction = MOZ_SIGNAL_TRAMPOLINE(SigprofHandler);
   sigemptyset(&sa.sa_mask);
   sa.sa_flags = SA_RESTART | SA_SIGINFO;
-  if (sigaction(SIGPROF, &sa, &gOldSigprofSignalHandler) != 0) {
+  if (sigaction(SIGPROF, &sa, &gOldSigprofHandler) != 0) {
     LOG("Error installing signal");
     return;
   }
   LOG("Signal installed");
-  gIsSigprofSignalHandlerInstalled = true;
+  gIsSigprofHandlerInstalled = true;
 
 #if defined(USE_LUL_STACKWALK)
   // Switch into unwind mode.  After this point, we can't add or
   // remove any unwind info to/from this LUL instance.  The only thing
   // we can do with it is Unwind() calls.
   gLUL->EnableUnwinding();
 
   // Has a test been requested?
@@ -407,46 +401,46 @@ PlatformStart()
   }
 #endif
 
   // Start a thread that sends SIGPROF signal to VM thread.
   // Sending the signal ourselves instead of relying on itimer provides
   // much better accuracy.
   MOZ_ASSERT(!gIsActive);
   gIsActive = true;
-  if (pthread_create(&gSignalSenderThread, NULL, SignalSender, NULL) == 0) {
-    gHasSignalSenderLaunched = true;
+  if (pthread_create(&gSigprofSenderThread, NULL, SigprofSender, NULL) == 0) {
+    gHasSigprofSenderLaunched = true;
   }
   LOG("Profiler thread started");
 }
 
 static void
 PlatformStop()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(gIsActive);
   gIsActive = false;
 
   // Wait for signal sender termination (it will exit after setting
   // active_ to false).
-  if (gHasSignalSenderLaunched) {
-    pthread_join(gSignalSenderThread, NULL);
-    gHasSignalSenderLaunched = false;
+  if (gHasSigprofSenderLaunched) {
+    pthread_join(gSigprofSenderThread, NULL);
+    gHasSigprofSenderLaunched = false;
   }
 
   // Restore old signal handler
-  if (gIsSigprofSignalHandlerInstalled) {
-    sigaction(SIGPROF, &gOldSigprofSignalHandler, 0);
-    gIsSigprofSignalHandlerInstalled = false;
+  if (gIsSigprofHandlerInstalled) {
+    sigaction(SIGPROF, &gOldSigprofHandler, 0);
+    gIsSigprofHandlerInstalled = false;
   }
 }
 
 #ifdef ANDROID
-static struct sigaction old_sigstart_signal_handler;
+static struct sigaction gOldSigstartHandler;
 const int SIGSTART = SIGUSR2;
 
 static void freeArray(const char** array, int size) {
   for (int i = 0; i < size; i++) {
     free((void*) array[i]);
   }
 }
 
@@ -543,32 +537,30 @@ static void StartSignalHandler(int signa
 
 void OS::Startup()
 {
   LOG("Registering start signal");
   struct sigaction sa;
   sa.sa_sigaction = StartSignalHandler;
   sigemptyset(&sa.sa_mask);
   sa.sa_flags = SA_RESTART | SA_SIGINFO;
-  if (sigaction(SIGSTART, &sa, &old_sigstart_signal_handler) != 0) {
+  if (sigaction(SIGSTART, &sa, &gOldSigstartHandler) != 0) {
     LOG("Error installing signal");
   }
 }
 
 #else
 
 void OS::Startup() {
   // Set up the fork handlers.
   setup_atfork();
 }
 
 #endif
 
-
-
 void TickSample::PopulateContext(void* aContext)
 {
   MOZ_ASSERT(aContext);
   ucontext_t* pContext = reinterpret_cast<ucontext_t*>(aContext);
   if (!getcontext(pContext)) {
     context = pContext;
     SetSampleContext(this, aContext);
   }
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -121,17 +121,17 @@ static mozilla::StaticMutex gThreadNameF
 // 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
-// SignalSender() in platform-linux.cc which is safe because there is implicit
+// SigprofSender() in platform-linux.cc which is safe because there is implicit
 // synchronization between that function and the set points in
 // profiler_{start,stop}().
 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);
--- a/tools/profiler/public/PseudoStack.h
+++ b/tools/profiler/public/PseudoStack.h
@@ -435,17 +435,17 @@ private:
   // been previously observed. This is used for an optimization: in some cases,
   // when a thread is asleep, we duplicate the previous sample, which is
   // cheaper than taking a new sample.
   //
   // mSleep is atomic because it is accessed from multiple threads.
   //
   // - It is written only by this thread, via setSleeping() and setAwake().
   //
-  // - It is read by the SamplerThread (on Win32 and Mac) or SignalSender
+  // - It is read by the SamplerThread (on Win32 and Mac) or the SigprofSender
   //   thread (on Linux and Android).
   //
   // There are two cases where racing between threads can cause an issue.
   //
   // - If CanDuplicateLastSampleDueToSleep() returns false but that result is
   //   invalidated before being acted upon, we will take a full sample
   //   unnecessarily. This is additional work but won't cause any correctness
   //   issues. (In actual fact, this case is impossible. In order to go from