Bug 1340928 (part 6) - Clean up profiler code relating to env vars. r=mstange.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 16 Feb 2017 13:59:35 +1100
changeset 373445 8ac1a3617dc977f6d4f36e644f41a1dd5acad6ff
parent 373444 7e6850af9372989b7dd66318076deb6df02ddf55
child 373446 e1ea30263e2f4a0c2339944c394fbf9138b099fe
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 6) - Clean up profiler code relating to env vars. r=mstange. This patch mostly does formatting fixes. It also removes some declarations from platform.h that are no longer necessary now that platform-linux-android.cpp is in the same compilation unit as platform.cpp (due to it being #include-d directly); this required reordering some things.
tools/profiler/core/platform.cpp
tools/profiler/core/platform.h
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -162,23 +162,16 @@ static Atomic<bool> gAddLeafAddresses(fa
 static Atomic<bool> gDisplayListDump(false);
 static Atomic<bool> gLayersDump(false);
 static Atomic<bool> gProfileGPU(false);
 static Atomic<bool> gProfileMemory(false);
 static Atomic<bool> gProfileRestyle(false);
 static Atomic<bool> gProfileThreads(false);
 static Atomic<bool> gUseStackWalk(false);
 
-// Environment variables to control the profiler
-const char* PROFILER_HELP = "MOZ_PROFILER_HELP";
-const char* PROFILER_INTERVAL = "MOZ_PROFILER_INTERVAL";
-const char* PROFILER_ENTRIES = "MOZ_PROFILER_ENTRIES";
-const char* PROFILER_STACK = "MOZ_PROFILER_STACK_SCAN";
-const char* PROFILER_FEATURES = "MOZ_PROFILING_FEATURES";
-
 /* we don't need to worry about overflow because we only treat the
  * case of them being the same as special. i.e. we only run into
  * a problem if 2^32 events happen between samples that we need
  * to know are associated with different events */
 
 // Values harvested from env vars, that control the profiler.
 static int gUnwindInterval;   /* in milliseconds */
 static int gUnwindStackScan;  /* max # of dubious frames allowed */
@@ -1248,136 +1241,116 @@ void ProfilerMarker::StreamJSON(Spliceab
           mPayload->StreamPayload(aWriter, aUniqueStacks);
       }
       aWriter.EndObject();
     }
   }
   aWriter.EndArray();
 }
 
-/* Has MOZ_PROFILER_VERBOSE been set? */
-
 // Verbosity control for the profiler.  The aim is to check env var
 // MOZ_PROFILER_VERBOSE only once.  However, we may need to temporarily
 // override that so as to print the profiler's help message.  That's
 // what profiler_set_verbosity is for.
 
-enum class ProfilerVerbosity : int8_t { UNCHECKED, NOTVERBOSE, VERBOSE };
+enum class Verbosity : int8_t { UNCHECKED, NOTVERBOSE, VERBOSE };
 
 // Raced on, potentially
-static ProfilerVerbosity profiler_verbosity = ProfilerVerbosity::UNCHECKED;
-
-bool profiler_verbose()
+static Verbosity gVerbosity = Verbosity::UNCHECKED;
+
+bool
+profiler_verbose()
 {
-  if (profiler_verbosity == ProfilerVerbosity::UNCHECKED) {
-    if (getenv("MOZ_PROFILER_VERBOSE") != nullptr)
-      profiler_verbosity = ProfilerVerbosity::VERBOSE;
-    else
-      profiler_verbosity = ProfilerVerbosity::NOTVERBOSE;
+  if (gVerbosity == Verbosity::UNCHECKED) {
+    gVerbosity = getenv("MOZ_PROFILER_VERBOSE")
+                       ? Verbosity::VERBOSE
+                       : Verbosity::NOTVERBOSE;
   }
 
-  return profiler_verbosity == ProfilerVerbosity::VERBOSE;
+  return gVerbosity == Verbosity::VERBOSE;
 }
 
-void profiler_set_verbosity(ProfilerVerbosity pv)
+static void
+profiler_set_verbosity(Verbosity aPv)
 {
-   MOZ_ASSERT(pv == ProfilerVerbosity::UNCHECKED ||
-              pv == ProfilerVerbosity::VERBOSE);
-   profiler_verbosity = pv;
+  MOZ_ASSERT(aPv == Verbosity::UNCHECKED ||
+             aPv == Verbosity::VERBOSE);
+  gVerbosity = aPv;
 }
 
-
-bool set_profiler_interval(const char* interval) {
-  if (interval) {
+static bool
+set_profiler_interval(const char* aInterval)
+{
+  if (aInterval) {
     errno = 0;
-    long int n = strtol(interval, (char**)nullptr, 10);
-    if (errno == 0 && n >= 1 && n <= 1000) {
+    long int n = strtol(aInterval, nullptr, 10);
+    if (errno == 0 && 1 <= n && n <= 1000) {
       gUnwindInterval = n;
       return true;
     }
     return false;
   }
 
   return true;
 }
 
-bool set_profiler_entries(const char* entries) {
-  if (entries) {
+static bool
+set_profiler_entries(const char* aEntries)
+{
+  if (aEntries) {
     errno = 0;
-    long int n = strtol(entries, (char**)nullptr, 10);
+    long int n = strtol(aEntries, nullptr, 10);
     if (errno == 0 && n > 0) {
       gProfileEntries = n;
       return true;
     }
     return false;
   }
 
   return true;
 }
 
-bool set_profiler_scan(const char* scanCount) {
-  if (scanCount) {
+static bool
+set_profiler_scan(const char* aScanCount)
+{
+  if (aScanCount) {
     errno = 0;
-    long int n = strtol(scanCount, (char**)nullptr, 10);
-    if (errno == 0 && n >= 0 && n <= 100) {
+    long int n = strtol(aScanCount, nullptr, 10);
+    if (errno == 0 && 0 <= n && n <= 100) {
       gUnwindStackScan = n;
       return true;
     }
     return false;
   }
 
   return true;
 }
 
-bool is_native_unwinding_avail() {
+static bool
+is_native_unwinding_avail()
+{
 # if defined(HAVE_NATIVE_UNWIND)
   return true;
 #else
   return false;
 #endif
 }
 
-// Read env vars at startup, so as to set:
-//   gUnwindInterval, gProfileEntries, gUnwindStackScan.
-void read_profiler_env_vars()
+// Environment variables to control the profiler
+static const char* PROFILER_HELP     = "MOZ_PROFILER_HELP";
+static const char* PROFILER_INTERVAL = "MOZ_PROFILER_INTERVAL";
+static const char* PROFILER_ENTRIES  = "MOZ_PROFILER_ENTRIES";
+static const char* PROFILER_STACK    = "MOZ_PROFILER_STACK_SCAN";
+#if defined(GP_OS_android)
+static const char* PROFILER_FEATURES = "MOZ_PROFILING_FEATURES";
+#endif
+
+static void
+profiler_usage()
 {
-  /* Set defaults */
-  gUnwindInterval = 0;  /* We'll have to look elsewhere */
-  gProfileEntries = 0;
-
-  const char* interval = getenv(PROFILER_INTERVAL);
-  const char* entries = getenv(PROFILER_ENTRIES);
-  const char* scanCount = getenv(PROFILER_STACK);
-
-  if (getenv(PROFILER_HELP)) {
-     // Enable verbose output
-     profiler_set_verbosity(ProfilerVerbosity::VERBOSE);
-     profiler_usage();
-     // Now force the next enquiry of profiler_verbose to re-query
-     // env var MOZ_PROFILER_VERBOSE.
-     profiler_set_verbosity(ProfilerVerbosity::UNCHECKED);
-  }
-
-  if (!set_profiler_interval(interval) ||
-      !set_profiler_entries(entries) ||
-      !set_profiler_scan(scanCount)) {
-      profiler_usage();
-  } else {
-    LOG( "Profiler:");
-    LOGF("Profiler: Sampling interval = %d ms (zero means \"platform default\")",
-        (int)gUnwindInterval);
-    LOGF("Profiler: Entry store size  = %d (zero means \"platform default\")",
-        (int)gProfileEntries);
-    LOGF("Profiler: UnwindStackScan   = %d (max dubious frames per unwind).",
-        (int)gUnwindStackScan);
-    LOG( "Profiler:");
-  }
-}
-
-void profiler_usage() {
   LOG( "Profiler: ");
   LOG( "Profiler: Environment variable usage:");
   LOG( "Profiler: ");
   LOG( "Profiler:   MOZ_PROFILER_HELP");
   LOG( "Profiler:   If set to any value, prints this message.");
   LOG( "Profiler: ");
   LOG( "Profiler:   MOZ_PROFILER_INTERVAL=<number>   (milliseconds, 1 to 1000)");
   LOG( "Profiler:   If unset, platform default is used.");
@@ -1407,66 +1380,101 @@ void profiler_usage() {
   LOG( "Profiler:");
   LOGF("Profiler: Sampling interval = %d ms (zero means \"platform default\")",
        (int)gUnwindInterval);
   LOGF("Profiler: Entry store size  = %d (zero means \"platform default\")",
        (int)gProfileEntries);
   LOGF("Profiler: UnwindStackScan   = %d (max dubious frames per unwind).",
        (int)gUnwindStackScan);
   LOG( "Profiler:");
-
-  return;
 }
 
-bool is_main_thread_name(const char* aName) {
-  if (!aName) {
-    return false;
+// Read env vars at startup, so as to set:
+//   gUnwindInterval, gProfileEntries, gUnwindStackScan.
+static void
+read_profiler_env_vars()
+{
+  /* Set defaults */
+  gUnwindInterval = 0;  /* We'll have to look elsewhere */
+  gProfileEntries = 0;
+
+  const char* interval = getenv(PROFILER_INTERVAL);
+  const char* entries = getenv(PROFILER_ENTRIES);
+  const char* scanCount = getenv(PROFILER_STACK);
+
+  if (getenv(PROFILER_HELP)) {
+    // Enable verbose output
+    profiler_set_verbosity(Verbosity::VERBOSE);
+    profiler_usage();
+    // Now force the next enquiry of profiler_verbose to re-query
+    // env var MOZ_PROFILER_VERBOSE.
+    profiler_set_verbosity(Verbosity::UNCHECKED);
   }
-  return strcmp(aName, gGeckoThreadName) == 0;
+
+  if (!set_profiler_interval(interval) ||
+      !set_profiler_entries(entries) ||
+      !set_profiler_scan(scanCount)) {
+      profiler_usage();
+  } else {
+    LOG( "Profiler:");
+    LOGF("Profiler: Sampling interval = %d ms (zero means \"platform default\")",
+        (int)gUnwindInterval);
+    LOGF("Profiler: Entry store size  = %d (zero means \"platform default\")",
+        (int)gProfileEntries);
+    LOGF("Profiler: UnwindStackScan   = %d (max dubious frames per unwind).",
+        (int)gUnwindStackScan);
+    LOG( "Profiler:");
+  }
+}
+
+static bool
+is_main_thread_name(const char* aName)
+{
+  return aName && (strcmp(aName, gGeckoThreadName) == 0);
 }
 
 #ifdef HAVE_VA_COPY
-#define VARARGS_ASSIGN(foo, bar)        VA_COPY(foo,bar)
+#define VARARGS_ASSIGN(foo, bar)     VA_COPY(foo,bar)
 #elif defined(HAVE_VA_LIST_AS_ARRAY)
 #define VARARGS_ASSIGN(foo, bar)     foo[0] = bar[0]
 #else
 #define VARARGS_ASSIGN(foo, bar)     (foo) = (bar)
 #endif
 
 void
-profiler_log(const char* str)
+profiler_log(const char* aStr)
 {
   // This function runs both on and off the main thread.
 
-  profiler_tracing("log", str, TRACING_EVENT);
+  profiler_tracing("log", aStr, TRACING_EVENT);
 }
 
 void
-profiler_log(const char* fmt, va_list args)
+profiler_log(const char* aFmt, va_list aArgs)
 {
   // This function runs both on and off the main thread.
 
   if (profiler_is_active()) {
     // nsAutoCString AppendPrintf would be nicer but
     // this is mozilla external code
     char buf[2048];
     va_list argsCpy;
-    VARARGS_ASSIGN(argsCpy, args);
-    int required = VsprintfLiteral(buf, fmt, argsCpy);
+    VARARGS_ASSIGN(argsCpy, aArgs);
+    int required = VsprintfLiteral(buf, aFmt, argsCpy);
     va_end(argsCpy);
 
     if (required < 0) {
       return; // silently drop for now
     } else if (required < 2048) {
       profiler_tracing("log", buf, TRACING_EVENT);
     } else {
       char* heapBuf = new char[required+1];
       va_list argsCpy;
-      VARARGS_ASSIGN(argsCpy, args);
-      vsnprintf(heapBuf, required+1, fmt, argsCpy);
+      VARARGS_ASSIGN(argsCpy, aArgs);
+      vsnprintf(heapBuf, required+1, aFmt, argsCpy);
       va_end(argsCpy);
       // EVENT_BACKTRACE could be used to get a source
       // for all log events. This could be a runtime
       // flag later.
       profiler_tracing("log", heapBuf, TRACING_EVENT);
       delete[] heapBuf;
     }
   }
--- a/tools/profiler/core/platform.h
+++ b/tools/profiler/core/platform.h
@@ -128,32 +128,16 @@ public:
 #if defined(MOZ_PROFILING) && \
     (defined(GP_OS_windows) || \
      defined(GP_OS_darwin) || \
      defined(GP_OS_linux) || \
      defined(GP_PLAT_arm_android))
 # define HAVE_NATIVE_UNWIND
 #endif
 
-/* Some values extracted at startup from environment variables, that
-   control the behaviour of the breakpad unwinder. */
-extern const char* PROFILER_INTERVAL;
-extern const char* PROFILER_ENTRIES;
-extern const char* PROFILER_STACK;
-extern const char* PROFILER_FEATURES;
-
-void read_profiler_env_vars();
-void profiler_usage();
-
-// Helper methods to expose modifying profiler behavior
-bool set_profiler_interval(const char*);
-bool set_profiler_entries(const char*);
-bool set_profiler_scan(const char*);
-bool is_native_unwinding_avail();
-
 // ----------------------------------------------------------------------------
 // Miscellaneous
 
 class PlatformData;
 
 // We can't new/delete the type safely without defining it
 // (-Wdelete-incomplete).  Use these to hide the details from clients.
 struct PlatformDataDestructor {