Bug 1499507 - Add ProfilingStackFrame flags for to choose the string template that is used to combine the label with the dynamic string. r=njn
authorMarkus Stange <mstange@themasta.com>
Tue, 06 Nov 2018 04:33:07 +0000
changeset 444532 2bc3310f933cb414575ab76435e5e62e8d6fdb82
parent 444531 be8a9a7f29bda128366fd1d08097fe34d8c717e2
child 444533 408a37a9e42b9fa9d219641c240a6cc98e9ea756
push id34996
push userrgurzau@mozilla.com
push dateTue, 06 Nov 2018 09:53:23 +0000
treeherdermozilla-central@e160f0a60e4f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1499507
milestone65.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 1499507 - Add ProfilingStackFrame flags for to choose the string template that is used to combine the label with the dynamic string. r=njn These flags will be used by WebIDL APIs in an upcoming patch. Depends on D9199 Differential Revision: https://phabricator.services.mozilla.com/D9203
js/public/ProfilingStack.h
tools/profiler/core/ProfileBuffer.cpp
tools/profiler/core/ProfileBuffer.h
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ProfileBufferEntry.h
tools/profiler/core/platform.cpp
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -147,17 +147,17 @@ class ProfilingStackFrame
 
     // The bytecode offset for JS stack frames.
     // Must not be used on non-JS frames; it'll contain either the default 0,
     // or a leftover value from a previous JS stack frame that was using this
     // ProfilingStackFrame object.
     mozilla::Atomic<int32_t, mozilla::ReleaseAcquire,
                     mozilla::recordreplay::Behavior::DontPreserve> pcOffsetIfJS_;
 
-    // Bits 0...3 hold the Flags. Bits 4...31 hold the category.
+    // Bits 0...6 hold the Flags. Bits 7...31 hold the category.
     mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire,
                     mozilla::recordreplay::Behavior::DontPreserve> flagsAndCategory_;
 
     static int32_t pcToOffset(JSScript* aScript, jsbytecode* aPc);
 
   public:
     ProfilingStackFrame() = default;
     ProfilingStackFrame& operator=(const ProfilingStackFrame& other)
@@ -168,18 +168,18 @@ class ProfilingStackFrame
         spOrScript = spScript;
         int32_t offsetIfJS = other.pcOffsetIfJS_;
         pcOffsetIfJS_ = offsetIfJS;
         uint32_t flagsAndCategory = other.flagsAndCategory_;
         flagsAndCategory_ = flagsAndCategory;
         return *this;
     }
 
-    // 4 bits for the flags.
-    // That leaves 32 - 4 = 28 bits for the category.
+    // 7 bits for the flags.
+    // That leaves 32 - 7 = 25 bits for the category.
     enum class Flags : uint32_t {
         // The first three flags describe the kind of the frame and are
         // mutually exclusive. (We still give them individual bits for
         // simplicity.)
 
         // A regular label frame. These usually come from AutoProfilerLabel.
         IS_LABEL_FRAME = 1 << 0,
 
@@ -193,17 +193,26 @@ class ProfilingStackFrame
         // A JS frame.
         IS_JS_FRAME = 1 << 2,
 
         // An interpreter JS frame that has OSR-ed into baseline. IS_JS_FRAME
         // frames can have this flag set and unset during their lifetime.
         // JS_OSR frames are ignored.
         JS_OSR = 1 << 3,
 
-        FLAGS_BITCOUNT = 4,
+        // The next three are mutually exclusive.
+        // By default, for profiling stack frames that have both a label and a
+        // dynamic string, the two strings are combined into one string of the
+        // form "<label> <dynamicString>" during JSON serialization. The
+        // following flags can be used to change this preset.
+        STRING_TEMPLATE_METHOD = 1 << 4, // "<label>.<dynamicString>"
+        STRING_TEMPLATE_GETTER = 1 << 5, // "get <label>.<dynamicString>"
+        STRING_TEMPLATE_SETTER = 1 << 6, // "set <label>.<dynamicString>"
+
+        FLAGS_BITCOUNT = 7,
         FLAGS_MASK = (1 << FLAGS_BITCOUNT) - 1
     };
 
     // Keep these in sync with devtools/client/performance/modules/categories.js
     enum class Category : uint32_t {
         IDLE,
         OTHER,
         LAYOUT,
@@ -287,16 +296,20 @@ class ProfilingStackFrame
         spOrScript = aScript;
         pcOffsetIfJS_ = pcToOffset(aScript, aPc);
         flagsAndCategory_ =
             uint32_t(Flags::IS_JS_FRAME) |
             (uint32_t(Category::JS) << uint32_t(Flags::FLAGS_BITCOUNT));
         MOZ_ASSERT(isJsFrame());
     }
 
+    uint32_t flags() const {
+        return uint32_t(flagsAndCategory_) & uint32_t(Flags::FLAGS_MASK);
+    }
+
     Category category() const {
         return Category(flagsAndCategory_ >> uint32_t(Flags::FLAGS_BITCOUNT));
     }
 
     void* stackAddress() const {
         MOZ_ASSERT(!isJsFrame());
         return spOrScript;
     }
--- a/tools/profiler/core/ProfileBuffer.cpp
+++ b/tools/profiler/core/ProfileBuffer.cpp
@@ -63,21 +63,22 @@ void
 ProfileBuffer::AddStoredMarker(ProfilerMarker *aStoredMarker)
 {
   aStoredMarker->SetPositionInBuffer(mRangeEnd);
   mStoredMarkers.insert(aStoredMarker);
 }
 
 void
 ProfileBuffer::CollectCodeLocation(
-  const char* aLabel, const char* aStr,
+  const char* aLabel, const char* aStr, uint32_t aFrameFlags,
   const Maybe<uint32_t>& aLineNumber, const Maybe<uint32_t>& aColumnNumber,
   const Maybe<js::ProfilingStackFrame::Category>& aCategory)
 {
   AddEntry(ProfileBufferEntry::Label(aLabel));
+  AddEntry(ProfileBufferEntry::FrameFlags(uint64_t(aFrameFlags)));
 
   if (aStr) {
     // Store the string using one or more DynamicStringFragment entries.
     size_t strLen = strlen(aStr) + 1;   // +1 for the null terminator
     for (size_t j = 0; j < strLen; ) {
       // Store up to kNumChars characters in the entry.
       char chars[ProfileBufferEntry::kNumChars];
       size_t len = ProfileBufferEntry::kNumChars;
@@ -149,17 +150,18 @@ void
 ProfileBufferCollector::CollectJitReturnAddr(void* aAddr)
 {
   mBuf.AddEntry(ProfileBufferEntry::JitReturnAddr(aAddr));
 }
 
 void
 ProfileBufferCollector::CollectWasmFrame(const char* aLabel)
 {
-  mBuf.CollectCodeLocation("", aLabel, Nothing(), Nothing(), Nothing());
+  mBuf.CollectCodeLocation("", aLabel, 0,
+                           Nothing(), Nothing(), Nothing());
 }
 
 void
 ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame& aFrame)
 {
   // WARNING: this function runs within the profiler's "critical section".
 
   MOZ_ASSERT(aFrame.isLabelFrame() ||
@@ -204,11 +206,11 @@ ProfileBufferCollector::CollectProfiling
     // Adjust the dynamic string as necessary.
     if (ProfilerFeature::HasPrivacy(mFeatures) && !isChromeJSEntry) {
       dynamicString = "(private)";
     } else if (strlen(dynamicString) >= ProfileBuffer::kMaxFrameKeyLength) {
       dynamicString = "(too long)";
     }
   }
 
-  mBuf.CollectCodeLocation(label, dynamicString, line, column,
-                           Some(aFrame.category()));
+  mBuf.CollectCodeLocation(label, dynamicString, aFrame.flags(),
+                           line, column, Some(aFrame.category()));
 }
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -39,17 +39,17 @@ public:
   // Add |aEntry| to the buffer, ignoring what kind of entry it is.
   void AddEntry(const ProfileBufferEntry& aEntry);
 
   // Add to the buffer a sample start (ThreadId) entry for aThreadId.
   // Returns the position of the entry.
   uint64_t AddThreadIdEntry(int aThreadId);
 
   void CollectCodeLocation(
-    const char* aLabel, const char* aStr,
+    const char* aLabel, const char* aStr, uint32_t aFrameFlags,
     const mozilla::Maybe<uint32_t>& aLineNumber,
     const mozilla::Maybe<uint32_t>& aColumnNumber,
     const mozilla::Maybe<js::ProfilingStackFrame::Category>& aCategory);
 
   // Maximum size of a frameKey string that we'll handle.
   static const size_t kMaxFrameKeyLength = 512;
 
   // Add JIT frame information to aJITFrameInfo for any JitReturnAddr entries
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -818,17 +818,17 @@ private:
 // The following grammar shows legal sequences of profile buffer entries.
 // The sequences beginning with a ThreadId entry are known as "samples".
 //
 // (
 //   ( /* Samples */
 //     ThreadId
 //     Time
 //     ( NativeLeafAddr
-//     | Label DynamicStringFragment* LineNumber? Category?
+//     | Label FrameFlags? DynamicStringFragment* LineNumber? Category?
 //     | JitReturnAddr
 //     )+
 //     Marker*
 //     Responsiveness?
 //     ResidentMemory?
 //     UnsharedMemory?
 //   )
 //   | ( ResidentMemory UnsharedMemory? Time)  /* Memory */
@@ -861,75 +861,82 @@ private:
 //
 //     Label("ElementRestyler::ComputeStyleChangeFor")
 //     LineNumber(3003)
 //     Category(ProfilingStackFrame::Category::CSS)
 //
 // - ProfilingStack frames with a dynamic string:
 //
 //     Label("nsObserverService::NotifyObservers")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_LABEL_FRAME))
 //     DynamicStringFragment("domwindo")
 //     DynamicStringFragment("wopened")
 //     LineNumber(291)
 //     Category(ProfilingStackFrame::Category::OTHER)
 //
 //     Label("")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_JS_FRAME))
 //     DynamicStringFragment("closeWin")
 //     DynamicStringFragment("dow (chr")
 //     DynamicStringFragment("ome://gl")
 //     DynamicStringFragment("obal/con")
 //     DynamicStringFragment("tent/glo")
 //     DynamicStringFragment("balOverl")
 //     DynamicStringFragment("ay.js:5)")
 //     DynamicStringFragment("")          # this string holds the closing '\0'
 //     LineNumber(25)
 //     Category(ProfilingStackFrame::Category::JS)
 //
 //     Label("")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_JS_FRAME))
 //     DynamicStringFragment("bound (s")
 //     DynamicStringFragment("elf-host")
 //     DynamicStringFragment("ed:914)")
 //     LineNumber(945)
 //     Category(ProfilingStackFrame::Category::JS)
 //
 // - A profiling stack frame with a dynamic string, but with privacy enabled:
 //
 //     Label("nsObserverService::NotifyObservers")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_LABEL_FRAME))
 //     DynamicStringFragment("(private")
 //     DynamicStringFragment(")")
 //     LineNumber(291)
 //     Category(ProfilingStackFrame::Category::OTHER)
 //
 // - A profiling stack frame with an overly long dynamic string:
 //
 //     Label("")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_LABEL_FRAME))
 //     DynamicStringFragment("(too lon")
 //     DynamicStringFragment("g)")
 //     LineNumber(100)
 //     Category(ProfilingStackFrame::Category::NETWORK)
 //
 // - A wasm JIT frame:
 //
 //     Label("")
+//     FrameFlags(uint64_t(0))
 //     DynamicStringFragment("wasm-fun")
 //     DynamicStringFragment("ction[87")
 //     DynamicStringFragment("36] (blo")
 //     DynamicStringFragment("b:http:/")
 //     DynamicStringFragment("/webasse")
 //     DynamicStringFragment("mbly.org")
 //     DynamicStringFragment("/3dc5759")
 //     DynamicStringFragment("4-ce58-4")
 //     DynamicStringFragment("626-975b")
 //     DynamicStringFragment("-08ad116")
 //     DynamicStringFragment("30bc1:38")
 //     DynamicStringFragment("29856)")
 //
 // - A JS frame in a synchronous sample:
 //
 //     Label("")
+//     FrameFlags(uint64_t(ProfilingStackFrame::Flags::IS_LABEL_FRAME))
 //     DynamicStringFragment("u (https")
 //     DynamicStringFragment("://perf-")
 //     DynamicStringFragment("html.io/")
 //     DynamicStringFragment("ac0da204")
 //     DynamicStringFragment("aaa44d75")
 //     DynamicStringFragment("a800.bun")
 //     DynamicStringFragment("dle.js:2")
 //     DynamicStringFragment("5)")
@@ -1020,18 +1027,24 @@ ProfileBuffer::StreamSamplesToJSON(Splic
         SprintfLiteral(buf, "%#llx", pc);
         stack = aUniqueStacks.AppendFrame(stack, UniqueStacks::FrameKey(buf));
         e.Next();
 
       } else if (e.Get().IsLabel()) {
         numFrames++;
 
         const char* label = e.Get().u.mString;
+        e.Next();
 
-        e.Next();
+        using FrameFlags = js::ProfilingStackFrame::Flags;
+        uint32_t frameFlags = 0;
+        if (e.Has() && e.Get().IsFrameFlags()) {
+          frameFlags = uint32_t(e.Get().u.mUint64);
+          e.Next();
+        }
 
         // Copy potential dynamic string fragments into dynStrBuf, so that
         // dynStrBuf will then contain the entire dynamic string.
         size_t i = 0;
         dynStrBuf[0] = '\0';
         while (e.Has()) {
           if (e.Get().IsDynamicStringFragment()) {
             for (size_t j = 0; j < ProfileBufferEntry::kNumChars; j++) {
@@ -1046,17 +1059,27 @@ ProfileBuffer::StreamSamplesToJSON(Splic
             break;
           }
         }
         dynStrBuf[kMaxFrameKeyLength - 1] = '\0';
         bool hasDynamicString = (i != 0);
 
         nsCString frameLabel;
         if (label[0] != '\0' && hasDynamicString) {
-          frameLabel.AppendPrintf("%s %s", label, dynStrBuf.get());
+          if (frameFlags & uint32_t(FrameFlags::STRING_TEMPLATE_METHOD)) {
+            frameLabel.AppendPrintf("%s.%s", label, dynStrBuf.get());
+          } else if (frameFlags & uint32_t(FrameFlags::STRING_TEMPLATE_GETTER)) {
+            frameLabel.AppendPrintf("get %s.%s", label, dynStrBuf.get());
+          } else if (frameFlags & uint32_t(FrameFlags::STRING_TEMPLATE_SETTER)) {
+            frameLabel.AppendPrintf("set %s.%s", label, dynStrBuf.get());
+          } else {
+            frameLabel.AppendPrintf("%s %s", label, dynStrBuf.get());
+          }
+        } else if (hasDynamicString) {
+          frameLabel.Append(dynStrBuf.get());
         } else {
           frameLabel.Append(label);
         }
 
         Maybe<unsigned> line;
         if (e.Has() && e.Get().IsLineNumber()) {
           line = Some(unsigned(e.Get().u.mInt));
           e.Next();
--- a/tools/profiler/core/ProfileBufferEntry.h
+++ b/tools/profiler/core/ProfileBufferEntry.h
@@ -32,16 +32,17 @@ class ProfilerMarker;
 
 // NOTE!  If you add entries, you need to verify if they need to be added to the
 // switch statement in DuplicateLastSample!
 #define FOR_EACH_PROFILE_BUFFER_ENTRY_KIND(macro) \
   macro(Category,              int) \
   macro(CollectionStart,       double) \
   macro(CollectionEnd,         double) \
   macro(Label,                 const char*) \
+  macro(FrameFlags,            uint64_t) \
   macro(DynamicStringFragment, char*) /* char[kNumChars], really */ \
   macro(JitReturnAddr,         void*) \
   macro(LineNumber,            int) \
   macro(ColumnNumber,          int) \
   macro(NativeLeafAddr,        void*) \
   macro(Marker,                ProfilerMarker*) \
   macro(Pause,                 double) \
   macro(Responsiveness,        double) \
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -1835,18 +1835,18 @@ CollectJavaThreadProfileData()
       if (frameNameString.EqualsLiteral("android.os.MessageQueue.nativePollOnce()")) {
         category = Some(js::ProfilingStackFrame::Category::IDLE);
         parentFrameWasIdleFrame = true;
       } else if (parentFrameWasIdleFrame) {
         category = Some(js::ProfilingStackFrame::Category::OTHER);
         parentFrameWasIdleFrame = false;
       }
 
-      buffer->CollectCodeLocation("", frameNameString.get(), Nothing(),
-          Nothing(), category);
+      buffer->CollectCodeLocation("", frameNameString.get(), 0,
+          Nothing(), Nothing(), category);
     }
     sampleId++;
   }
   return buffer;
 }
 #endif
 
 static void