Bug 1586920 - Sometimes include dynamic string of label frames in BHR r=nika
authorDoug Thayer <dothayer@mozilla.com>
Mon, 11 Nov 2019 20:27:44 +0000
changeset 501846 bd28a94055a30b1bb460d2735407ffa97c3fbcba
parent 501845 9e55fee783ff8a1dc0c512316689145a57f8b6aa
child 501847 60db8c551c38d0619df981ac11e5616b8d8e3002
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1586920
milestone72.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 1586920 - Sometimes include dynamic string of label frames in BHR r=nika This adds two AUTO_PROFILER_LABEL_DYNAMIC_... macros and updates select usages of the old macros to use the new ones. These new macros cause the dynamic string of the label to be included in BHR stacks. We don't want to do this all of the time, as in many cases we may not be interested enough in the dynamic string or it may be sensitive information, but it is rather important information for certain cases. This uses the same buffer that we use for the strings for JS frames, and if we fail to fit into that buffer we just append the raw label. If the string is too long for our static buffer (128 bytes), we just leave it truncated, as it should be stable and we may be able to infer from the truncated form what the full form would be. Differential Revision: https://phabricator.services.mozilla.com/D51665
dom/base/ChromeUtils.cpp
dom/base/nsJSEnvironment.cpp
js/public/ProfilingStack.h
js/xpconnect/loader/mozJSSubScriptLoader.cpp
layout/base/PresShell.cpp
layout/painting/FrameLayerBuilder.cpp
toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp
toolkit/components/backgroundhangmonitor/ThreadStackHelper.h
toolkit/components/telemetry/docs/data/backgroundhangmonitor-ping.rst
tools/profiler/public/GeckoProfiler.h
xpcom/ds/nsObserverService.cpp
--- a/dom/base/ChromeUtils.cpp
+++ b/dom/base/ChromeUtils.cpp
@@ -407,18 +407,18 @@ void ChromeUtils::Import(const GlobalObj
                          const Optional<JS::Handle<JSObject*>>& aTargetObj,
                          JS::MutableHandle<JSObject*> aRetval,
                          ErrorResult& aRv) {
   RefPtr<mozJSComponentLoader> moduleloader = mozJSComponentLoader::Get();
   MOZ_ASSERT(moduleloader);
 
   NS_ConvertUTF16toUTF8 registryLocation(aResourceURI);
 
-  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING("ChromeUtils::Import", OTHER,
-                                        registryLocation);
+  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE("ChromeUtils::Import",
+                                                     OTHER, registryLocation);
 
   JSContext* cx = aGlobal.Context();
 
   bool ignoreExports = aTargetObj.WasPassed() && !aTargetObj.Value();
 
   JS::RootedObject global(cx);
   JS::RootedObject exports(cx);
   nsresult rv = moduleloader->Import(cx, registryLocation, &global, &exports,
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -1146,18 +1146,18 @@ void nsJSContext::SetLowMemoryState(bool
   JS::SetLowMemoryState(cx, aState);
 }
 
 // static
 void nsJSContext::GarbageCollectNow(JS::GCReason aReason,
                                     IsIncremental aIncremental,
                                     IsShrinking aShrinking,
                                     int64_t aSliceMillis) {
-  AUTO_PROFILER_LABEL_DYNAMIC_CSTR("nsJSContext::GarbageCollectNow", GCCC,
-                                   JS::ExplainGCReason(aReason));
+  AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(
+      "nsJSContext::GarbageCollectNow", GCCC, JS::ExplainGCReason(aReason));
 
   MOZ_ASSERT_IF(aSliceMillis, aIncremental == IncrementalGC);
 
   KillGCTimer();
 
   // We use danger::GetJSContext() since AutoJSAPI will assert if the current
   // thread's context is null (such as during shutdown).
   JSContext* cx = danger::GetJSContext();
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -206,30 +206,36 @@ class ProfilingStackFrame {
     // the profile JSON, which will make it show up in the "JS only" call
     // tree view.
     RELEVANT_FOR_JS = 1 << 7,
 
     // If set, causes the label on this ProfilingStackFrame to be ignored
     // and to be replaced by the subcategory's label.
     LABEL_DETERMINED_BY_CATEGORY_PAIR = 1 << 8,
 
-    FLAGS_BITCOUNT = 9,
+    NONSENSITIVE = 1 << 9,
+
+    FLAGS_BITCOUNT = 10,
     FLAGS_MASK = (1 << FLAGS_BITCOUNT) - 1
   };
 
   static_assert(
       uint32_t(JS::ProfilingCategoryPair::LAST) <=
           (UINT32_MAX >> uint32_t(Flags::FLAGS_BITCOUNT)),
       "Too many category pairs to fit into u32 with together with the "
       "reserved bits for the flags");
 
   bool isLabelFrame() const {
     return uint32_t(flagsAndCategoryPair_) & uint32_t(Flags::IS_LABEL_FRAME);
   }
 
+  bool isNonsensitive() const {
+    return uint32_t(flagsAndCategoryPair_) & uint32_t(Flags::NONSENSITIVE);
+  }
+
   bool isSpMarkerFrame() const {
     return uint32_t(flagsAndCategoryPair_) &
            uint32_t(Flags::IS_SP_MARKER_FRAME);
   }
 
   bool isJsFrame() const {
     return uint32_t(flagsAndCategoryPair_) & uint32_t(Flags::IS_JS_FRAME);
   }
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -396,17 +396,17 @@ nsresult mozJSSubScriptLoader::DoLoadSub
   if (!serv) {
     ReportError(cx, NS_LITERAL_CSTRING(LOAD_ERROR_NOSERVICE));
     return NS_OK;
   }
 
   NS_LossyConvertUTF16toASCII asciiUrl(url);
   AUTO_PROFILER_TEXT_MARKER_CAUSE("SubScript", asciiUrl, JS,
                                   profiler_get_backtrace());
-  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
+  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE(
       "mozJSSubScriptLoader::DoLoadSubScriptWithOptions", OTHER, asciiUrl);
 
   // Make sure to explicitly create the URI, since we'll need the
   // canonicalized spec.
   rv = NS_NewURI(getter_AddRefs(uri), asciiUrl);
   if (NS_FAILED(rv)) {
     ReportError(cx, NS_LITERAL_CSTRING(LOAD_ERROR_NOURI));
     return NS_OK;
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3950,18 +3950,19 @@ void PresShell::DoFlushPendingNotificati
     // Frames are the same
     "Style",
     "Style",
     "InterruptibleLayout",
     "Layout",
     "Display"
   };
   // clang-format on
-  AUTO_PROFILER_LABEL_DYNAMIC_CSTR("PresShell::DoFlushPendingNotifications",
-                                   LAYOUT, flushTypeNames[flushType]);
+  AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(
+      "PresShell::DoFlushPendingNotifications", LAYOUT,
+      flushTypeNames[flushType]);
 #endif
 
 #ifdef ACCESSIBILITY
 #  ifdef DEBUG
   if (nsAccessibilityService* accService = GetAccService()) {
     NS_ASSERTION(!accService->IsProcessingRefreshDriverNotification(),
                  "Flush during accessible tree update!");
   }
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -7000,18 +7000,18 @@ void FrameLayerBuilder::PaintItems(std::
 
       // Sometimes the item that was going to reuse the previous clip is culled.
       // Since |PushClip()| is never called for culled items, pop the clip now.
       effectClipStack.PopDeferredClip();
       continue;
     }
 
 #ifdef MOZ_DUMP_PAINTING
-    AUTO_PROFILER_LABEL_DYNAMIC_CSTR("FrameLayerBuilder::PaintItems",
-                                     GRAPHICS_Rasterization, item->Name());
+    AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(
+        "FrameLayerBuilder::PaintItems", GRAPHICS_Rasterization, item->Name());
 #else
     AUTO_PROFILER_LABEL("FrameLayerBuilder::PaintItems",
                         GRAPHICS_Rasterization);
 #endif
 
     MOZ_ASSERT((opacityLevel == 0 && !cdi.HasOpacity()) ||
                (opacityLevel > 0 && cdi.HasOpacity()) ||
                (transformLevel == 0 && !cdi.HasTransform()) ||
--- a/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp
+++ b/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp
@@ -61,16 +61,22 @@
 #  endif
 #  ifndef SYS_rt_tgsigqueueinfo
 #    define SYS_rt_tgsigqueueinfo __NR_rt_tgsigqueueinfo
 #  endif
 #endif
 
 namespace mozilla {
 
+// A character which we append to any string which gets truncated as a a
+// result of trying to write it into a statically allocated buffer. This just
+// makes it a little easier to know that the buffer was truncated during
+// analysis.
+const char kTruncationIndicator = '$';
+
 ThreadStackHelper::ThreadStackHelper()
     : mStackToFill(nullptr),
       mMaxStackSize(16),
       mMaxBufferSize(512),
       mDesiredStackSize(0),
       mDesiredBufferSize(0) {
   mThreadId = profiler_current_thread_id();
 }
@@ -241,21 +247,51 @@ const char* GetPathAfterComponent(const 
     // Resume searching before the separator '/'.
     next = strstr(found - 1, component);
   }
   return found;
 }
 
 }  // namespace
 
+bool ThreadStackHelper::MaybeAppendDynamicStackFrame(Span<const char> aBuf) {
+  mDesiredBufferSize += aBuf.Length() + 1;
+
+  if (mStackToFill->stack().Capacity() > mStackToFill->stack().Length() &&
+      (mStackToFill->strbuffer().Capacity() -
+       mStackToFill->strbuffer().Length()) > aBuf.Length() + 1) {
+    // NOTE: We only increment this if we're going to successfully append.
+    mDesiredStackSize += 1;
+    uint32_t start = mStackToFill->strbuffer().Length();
+    mStackToFill->strbuffer().AppendElements(aBuf.Elements(), aBuf.Length());
+    mStackToFill->strbuffer().AppendElement('\0');
+    mStackToFill->stack().AppendElement(HangEntryBufOffset(start));
+    return true;
+  }
+  return false;
+}
+
 void ThreadStackHelper::CollectProfilingStackFrame(
     const js::ProfilingStackFrame& aFrame) {
-  // For non-js frames we just include the raw label.
+  // For non-js frames, first try to get the dynamic string and fit it in,
+  // otherwise just get the label.
   if (!aFrame.isJsFrame()) {
     const char* frameLabel = aFrame.label();
+    if (aFrame.isNonsensitive() && aFrame.dynamicString()) {
+      const char* dynamicString = aFrame.dynamicString();
+      char buffer[128];
+      size_t len = SprintfLiteral(buffer, "%s %s", frameLabel, dynamicString);
+      if (len > sizeof(buffer)) {
+        buffer[sizeof(buffer) - 1] = kTruncationIndicator;
+        len = sizeof(buffer);
+      }
+      if (MaybeAppendDynamicStackFrame(MakeSpan(buffer, len))) {
+        return;
+      }
+    }
 
     // frameLabel is a statically allocated string, so we want to store a
     // reference to it without performing any allocations. This is important, as
     // we aren't allowed to allocate within this function.
     //
     // The variant for this kind of label in our HangStack object is a
     // `nsCString`, which normally contains heap allocated string data. However,
     // `nsCString` has an optimization for literal strings which causes the
@@ -319,28 +355,20 @@ void ThreadStackHelper::CollectProfiling
     filename = strrchr(basename, '\\');
     if (filename) {
       basename = filename + 1;
     }
   }
 
   char buffer[128];  // Enough to fit longest js file name from the tree
   size_t len = SprintfLiteral(buffer, "%s:%u", basename, lineno);
-  if (len < sizeof(buffer)) {
-    mDesiredBufferSize += len + 1;
-
-    if (mStackToFill->stack().Capacity() > mStackToFill->stack().Length() &&
-        (mStackToFill->strbuffer().Capacity() -
-         mStackToFill->strbuffer().Length()) > len + 1) {
-      // NOTE: We only increment this if we're going to successfully append.
-      mDesiredStackSize += 1;
-      uint32_t start = mStackToFill->strbuffer().Length();
-      mStackToFill->strbuffer().AppendElements(buffer, len);
-      mStackToFill->strbuffer().AppendElement('\0');
-      mStackToFill->stack().AppendElement(HangEntryBufOffset(start));
-      return;
-    }
+  if (len > sizeof(buffer)) {
+    buffer[sizeof(buffer) - 1] = kTruncationIndicator;
+    len = sizeof(buffer);
+  }
+  if (MaybeAppendDynamicStackFrame(MakeSpan(buffer, len))) {
+    return;
   }
 
   TryAppendFrame(HangEntryChromeScript());
 }
 
 }  // namespace mozilla
--- a/toolkit/components/backgroundhangmonitor/ThreadStackHelper.h
+++ b/toolkit/components/backgroundhangmonitor/ThreadStackHelper.h
@@ -6,16 +6,17 @@
 
 #ifndef mozilla_ThreadStackHelper_h
 #define mozilla_ThreadStackHelper_h
 
 #ifdef MOZ_GECKO_PROFILER
 
 #  include "js/ProfilingStack.h"
 #  include "HangDetails.h"
+#  include "mozilla/Span.h"
 #  include "nsThread.h"
 
 #  include <stddef.h>
 
 #  if defined(XP_LINUX)
 #    include <signal.h>
 #    include <semaphore.h>
 #    include <sys/types.h>
@@ -90,16 +91,17 @@ class ThreadStackHelper : public Profile
   virtual void SetIsMainThread() override;
   virtual void CollectNativeLeafAddr(void* aAddr) override;
   virtual void CollectJitReturnAddr(void* aAddr) override;
   virtual void CollectWasmFrame(const char* aLabel) override;
   virtual void CollectProfilingStackFrame(
       const js::ProfilingStackFrame& aEntry) override;
 
  private:
+  bool MaybeAppendDynamicStackFrame(mozilla::Span<const char> aBuf);
   void TryAppendFrame(mozilla::HangEntry aFrame);
 
   // The profiler's unique thread identifier for the target thread.
   int mThreadId;
 };
 
 }  // namespace mozilla
 
--- a/toolkit/components/telemetry/docs/data/backgroundhangmonitor-ping.rst
+++ b/toolkit/components/telemetry/docs/data/backgroundhangmonitor-ping.rst
@@ -85,17 +85,40 @@ The structure that manages the label sta
 "PseudoStack" in the past and is now called "ProfilingStack".
 
 Note that this field only contains native stack frames, label stack and chrome
 JS script frames. If native stacks can not be collected on the target platform,
 or stackwalking was not initialized, there will be no native frames present, and
 the stack will consist only of label stack and chrome JS script frames.
 
 A single frame in the stack is either a raw string, representing a label stack
-or chrome JS script frame, or a native stack frame:
+or chrome JS script frame, or a native stack frame.
+
+Label stack frames contain the static string from all insances of the
+AUTO_PROFILER_LABEL* macros. Additionally, dynamic strings are collected from
+all usages of the AUTO_PROFILER_LABEL_DYNAMIC*_NONSENSITIVE macros. The dynamic
+strings are simply appended to the static strings after a space character.
+
+Current dynamic string collections are as follows:
+
++--------------------------------------------------+-----------------------------------------+
+| Static string                                    | Dynamic                                 |
++==================================================+=========================================+
+| ChromeUtils::Import                              | Associated chrome:// or resource:// URI |
++--------------------------------------------------+-----------------------------------------+
+| nsJSContext::GarbageCollectNow                   | GC reason string                        |
++--------------------------------------------------+-----------------------------------------+
+| mozJSSubScriptLoader::DoLoadSubScriptWithOptions | Associated chrome:// or resource:// URI |
++--------------------------------------------------+-----------------------------------------+
+| PresShell::DoFlushPendingNotifications           | Flush type                              |
++--------------------------------------------------+-----------------------------------------+
+| nsObserverService::NotifyObservers               | Associated observer topic               |
++--------------------------------------------------+-----------------------------------------+
+
+Native stack frames are as such:
 
 .. code-block:: js
 
     [
       <number>, // Index in the payload.modules list of the module description.
                 // -1 if this frame was not in a valid module.
       <string> // Hex string (e.g. "FF0F") of the frame offset in the module.
     ]
--- a/tools/profiler/public/GeckoProfiler.h
+++ b/tools/profiler/public/GeckoProfiler.h
@@ -42,17 +42,21 @@
 #  define PROFILER_JS_INTERRUPT_CALLBACK()
 
 #  define PROFILER_SET_JS_CONTEXT(cx)
 #  define PROFILER_CLEAR_JS_CONTEXT()
 
 #  define AUTO_PROFILER_LABEL(label, categoryPair)
 #  define AUTO_PROFILER_LABEL_CATEGORY_PAIR(categoryPair)
 #  define AUTO_PROFILER_LABEL_DYNAMIC_CSTR(label, categoryPair, cStr)
+#  define AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(label, categoryPair, \
+                                                        cStr)
 #  define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(label, categoryPair, nsCStr)
+#  define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE( \
+      label, categoryPair, nsCStr)
 #  define AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(label, categoryPair, nsStr)
 #  define AUTO_PROFILER_LABEL_FAST(label, categoryPair, ctx)
 #  define AUTO_PROFILER_LABEL_DYNAMIC_FAST(label, dynamicString, categoryPair, \
                                            ctx, flags)
 
 #  define PROFILER_ADD_MARKER(markerName, categoryPair)
 #  define PROFILER_ADD_MARKER_WITH_PAYLOAD(markerName, categoryPair, \
                                            PayloadType, payloadArgs)
@@ -680,32 +684,53 @@ mozilla::Maybe<ProfilerBufferInfo> profi
 // AUTO_PROFILER_LABEL are sampled, no string copy needs to be made because the
 // profile buffer can just store the raw pointers to the literal strings.
 // Consequently, AUTO_PROFILER_LABEL frames take up considerably less space in
 // the profile buffer than AUTO_PROFILER_LABEL_DYNAMIC_* frames.
 #  define AUTO_PROFILER_LABEL_DYNAMIC_CSTR(label, categoryPair, cStr) \
     mozilla::AutoProfilerLabel PROFILER_RAII(                         \
         label, cStr, JS::ProfilingCategoryPair::categoryPair)
 
+// Like AUTO_PROFILER_LABEL_DYNAMIC_CSTR, but with the NONSENSITIVE flag to
+// note that it does not contain sensitive information (so we can include it
+// in, for example, the BackgroundHangMonitor)
+#  define AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(label, categoryPair, \
+                                                        cStr)                \
+    mozilla::AutoProfilerLabel PROFILER_RAII(                                \
+        label, cStr, JS::ProfilingCategoryPair::categoryPair,                \
+        uint32_t(js::ProfilingStackFrame::Flags::NONSENSITIVE))
+
 // Similar to AUTO_PROFILER_LABEL_DYNAMIC_CSTR, but takes an nsACString.
 //
 // Note: The use of the Maybe<>s ensures the scopes for the dynamic string and
 // the AutoProfilerLabel are appropriate, while also not incurring the runtime
 // cost of the string assignment unless the profiler is active. Therefore,
 // unlike AUTO_PROFILER_LABEL and AUTO_PROFILER_LABEL_DYNAMIC_CSTR, this macro
 // doesn't push/pop a label when the profiler is inactive.
 #  define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(label, categoryPair, nsCStr) \
     mozilla::Maybe<nsAutoCString> autoCStr;                                  \
     mozilla::Maybe<mozilla::AutoProfilerLabel> raiiObjectNsCString;          \
     if (profiler_is_active()) {                                              \
       autoCStr.emplace(nsCStr);                                              \
       raiiObjectNsCString.emplace(label, autoCStr->get(),                    \
                                   JS::ProfilingCategoryPair::categoryPair);  \
     }
 
+// See note above AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE
+#  define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE(              \
+      label, categoryPair, nsCStr)                                         \
+    mozilla::Maybe<nsAutoCString> autoCStr;                                \
+    mozilla::Maybe<mozilla::AutoProfilerLabel> raiiObjectNsCString;        \
+    if (profiler_is_active()) {                                            \
+      autoCStr.emplace(nsCStr);                                            \
+      raiiObjectNsCString.emplace(                                         \
+          label, autoCStr->get(), JS::ProfilingCategoryPair::categoryPair, \
+          uint32_t(js::ProfilingStackFrame::Flags::NONSENSITIVE));         \
+    }
+
 // Similar to AUTO_PROFILER_LABEL_DYNAMIC_CSTR, but takes an nsString that is
 // is lossily converted to an ASCII string.
 //
 // Note: The use of the Maybe<>s ensures the scopes for the converted dynamic
 // string and the AutoProfilerLabel are appropriate, while also not incurring
 // the runtime cost of the string conversion unless the profiler is active.
 // Therefore, unlike AUTO_PROFILER_LABEL and AUTO_PROFILER_LABEL_DYNAMIC_CSTR,
 // this macro doesn't push/pop a label when the profiler is inactive.
--- a/xpcom/ds/nsObserverService.cpp
+++ b/xpcom/ds/nsObserverService.cpp
@@ -278,18 +278,18 @@ NS_IMETHODIMP nsObserverService::NotifyO
   if (NS_WARN_IF(!aTopic)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   mozilla::TimeStamp start = TimeStamp::Now();
 
   AUTO_PROFILER_TEXT_MARKER_CAUSE("NotifyObservers", nsDependentCString(aTopic),
                                   OTHER, profiler_get_backtrace());
-  AUTO_PROFILER_LABEL_DYNAMIC_CSTR("nsObserverService::NotifyObservers", OTHER,
-                                   aTopic);
+  AUTO_PROFILER_LABEL_DYNAMIC_CSTR_NONSENSITIVE(
+      "nsObserverService::NotifyObservers", OTHER, aTopic);
 
   nsObserverList* observerList = mObserverTopicTable.GetEntry(aTopic);
   if (observerList) {
     observerList->NotifyObservers(aSubject, aTopic, aSomeData);
   }
 
   uint32_t latencyMs = round((TimeStamp::Now() - start).ToMilliseconds());
   if (latencyMs >= kMinTelemetryNotifyObserversLatencyMs) {