Bug 855977 - SPS breakpad: disable stack scanning for profiling unwinds. r=bgirard

--HG--
extra : rebase_source : 6735671ab9f4b9e9ce4fc25238cb6c3f01ef4b02

tools/profiler/BreakpadSampler.cpp

@@ -51,8 +51,9 @@
 
 /* These will be set to something sensible before we take the first
    sample. */
-UnwMode sUnwindMode     = UnwINVALID;
-int     sUnwindInterval = 0;
+UnwMode sUnwindMode      = UnwINVALID;
+int     sUnwindInterval  = 0;
+int     sUnwindStackScan = 0;
 
 using std::string;
 using namespace mozilla;

tools/profiler/TableTicker.h

@@ -15,11 +15,6 @@ hasFeature(const char** aFeatures, uint32_t aFeatureCount, const char* aFeature)
   return false;
 }
 
-void read_env_vars();
-typedef  enum { UnwINVALID, UnwNATIVE, UnwPSEUDO, UnwCOMBINED }  UnwMode;
-extern UnwMode sUnwindMode;
-extern int     sUnwindInterval;
-
 extern TimeStamp sLastTracerEvent;
 extern int sFrameNumber;
 extern int sLastFrameNumber;

tools/profiler/UnwinderThread2.cpp

@@ -1011,6 +1011,11 @@ static void* unwind_thr_fn(void* exit_nowV)
           do_breakpad_unwind_Buffer(&pairs, &nPairs, buff, oldest_ix);
           buff->aProfile->addTag( ProfileEntry('s', "(root)") );
           for (unsigned int i = 0; i < nPairs; i++) {
+            /* Skip any outermost frames that
+               do_breakpad_unwind_Buffer didn't give us.  See comments
+               on that function for details. */
+            if (pairs[i].pc == 0 && pairs[i].sp == 0)
+              continue;
             buff->aProfile
                 ->addTag( ProfileEntry('l', reinterpret_cast<void*>(pairs[i].pc)) );
           }
@@ -1082,6 +1087,12 @@ static void* unwind_thr_fn(void* exit_nowV)
       if (0) LOGF("at mergeloop: n_pairs %llu ix_last_hQ %llu",
                   (unsigned long long int)n_pairs,
                   (unsigned long long int)ix_last_hQ);
+      /* Skip any outermost frames that do_breakpad_unwind_Buffer
+         didn't give us.  See comments on that function for
+         details. */
+      while (next_N < n_pairs && pairs[next_N].pc == 0 && pairs[next_N].sp == 0)
+        next_N++;
+
       while (true) {
         if (next_P <= ix_last_hQ) {
           // Assert that next_P points at the start of an P entry
@@ -1484,7 +1495,10 @@ class MyCodeModules : public google_breakpad::CodeModules
    responsible for deallocating it.
 
    The first pair is for the outermost frame, the last for the
-   innermost frame.
+   innermost frame.  There may be some leading section of the array
+   containing (zero, zero) values, in the case where the stack got
+   truncated because breakpad started stack-scanning, or for whatever
+   reason.  Users of this function need to be aware of that.
 */
 
 MyCodeModules* sModules = NULL;
@@ -1616,8 +1630,9 @@ void do_breakpad_unwind_Buffer(/*OUT*/PCandSP** pairs,
 
   unsigned int n_frames = stack->frames()->size();
   unsigned int n_frames_good = 0;
+  unsigned int n_frames_dubious = 0;
 
-  *pairs  = (PCandSP*)malloc(n_frames * sizeof(PCandSP));
+  *pairs  = (PCandSP*)calloc(n_frames, sizeof(PCandSP));
   *nPairs = n_frames;
   if (*pairs == NULL) {
     *nPairs = 0;
@@ -1629,11 +1644,26 @@ void do_breakpad_unwind_Buffer(/*OUT*/PCandSP** pairs,
          frame_index < n_frames; ++frame_index) {
       google_breakpad::StackFrame *frame = stack->frames()->at(frame_index);
 
-      if (frame->trust == google_breakpad::StackFrame::FRAME_TRUST_CFI
-          || frame->trust == google_breakpad::StackFrame::FRAME_TRUST_CONTEXT) {
+      bool dubious
+        = frame->trust == google_breakpad::StackFrame::FRAME_TRUST_SCAN
+          || frame->trust == google_breakpad::StackFrame::FRAME_TRUST_CFI_SCAN
+          || frame->trust == google_breakpad::StackFrame::FRAME_TRUST_NONE;
+
+      if (dubious) {
+        n_frames_dubious++;
+      } else {
         n_frames_good++;
       }
 
+      /* Once we've seen more than some threshhold number of dubious
+         frames, give up.  Doing that gives better results than
+         polluting the profiling results with junk frames.  Because
+         the entries are put into the pairs array starting at the end,
+         this will leave some initial section of pairs containing
+         (0,0) values, which correspond to the skipped frames. */
+      if (n_frames_dubious > (unsigned int)sUnwindStackScan)
+        break;
+
 #     if defined(SPS_ARCH_amd64)
       google_breakpad::StackFrameAMD64* frame_amd64
         = reinterpret_cast<google_breakpad::StackFrameAMD64*>(frame);

tools/profiler/platform.cpp

@@ -116,7 +116,7 @@ static inline const char* name_UnwMode(UnwMode m)
 }
 
 // Read env vars at startup, so as to set sUnwindMode and sInterval.
-void read_env_vars()
+void read_profiler_env_vars()
 {
   bool nativeAvail = false;
 # if defined(HAVE_NATIVE_UNWIND)
@@ -132,6 +132,7 @@ void read_env_vars()
 
   const char* strM = PR_GetEnv("MOZ_PROFILER_MODE");
   const char* strI = PR_GetEnv("MOZ_PROFILER_INTERVAL");
+  const char* strF = PR_GetEnv("MOZ_PROFILER_STACK_SCAN");
 
   if (strM) {
     if (0 == strcmp(strM, "pseudo"))
@@ -152,6 +153,15 @@ void read_env_vars()
     else goto usage;
   }
 
+  if (strF) {
+    errno = 0;
+    long int n = strtol(strF, (char**)NULL, 10);
+    if (errno == 0 && n >= 0 && n <= 100) {
+      sUnwindStackScan = n;
+    }
+    else goto usage;
+  }
+
   goto out;
 
  usage:
@@ -170,6 +180,9 @@ void read_env_vars()
   LOG( "SPS:   MOZ_PROFILER_VERBOSE");
   LOG( "SPS:   If set to any value, increases verbosity (recommended).");
   LOG( "SPS: ");
+  LOG( "SPS:   MOZ_PROFILER_STACK_SCAN=<number>   (default is zero)");
+  LOG( "SPS:   The number of dubious (stack-scanned) frames allowed");
+  LOG( "SPS: ");
   LOG( "SPS:   MOZ_PROFILER_NEW");
   LOG( "SPS:   Needs to be set to use Breakpad-based unwinding.");
   LOG( "SPS: ");
@@ -179,12 +192,15 @@ void read_env_vars()
   /* Re-set defaults */
   sUnwindMode       = nativeAvail ? UnwCOMBINED : UnwPSEUDO;
   sUnwindInterval   = 0;  /* We'll have to look elsewhere */
+  sUnwindStackScan  = 0;
 
  out:
   LOG( "SPS:");
   LOGF("SPS: Unwind mode       = %s", name_UnwMode(sUnwindMode));
   LOGF("SPS: Sampling interval = %d ms (zero means \"platform default\")",
        (int)sUnwindInterval);
+  LOGF("SPS: UnwindStackScan   = %d (max dubious frames per unwind).",
+       (int)sUnwindStackScan);
   LOG( "SPS: Use env var MOZ_PROFILER_MODE=help for further information.");
   LOG( "SPS:");
 
@@ -211,8 +227,9 @@ void mozilla_sampler_init()
 
   if (sps_version2()) {
     // Read mode settings from MOZ_PROFILER_MODE and interval
-    // settings from MOZ_PROFILER_INTERVAL.
-    read_env_vars();
+    // settings from MOZ_PROFILER_INTERVAL and stack-scan threshhold
+    // from MOZ_PROFILER_STACK_SCAN.
+    read_profiler_env_vars();
 
     // Create the unwinder thread.  ATM there is only one.
     uwt__init();

tools/profiler/platform.h

@@ -216,6 +216,15 @@ class Thread {
 # define HAVE_NATIVE_UNWIND
 #endif
 
+/* Some values extracted at startup from environment variables, that
+   control the behaviour of the breakpad unwinder. */
+void read_profiler_env_vars();
+typedef  enum { UnwINVALID, UnwNATIVE, UnwPSEUDO, UnwCOMBINED }  UnwMode;
+extern UnwMode sUnwindMode;       /* what mode? */
+extern int     sUnwindInterval;   /* in milliseconds */
+extern int     sUnwindStackScan;  /* max # of dubious frames allowed */
+
+
 // ----------------------------------------------------------------------------
 // Sampler
 //