Bug 1499507 - Convert the ProfilingStackFrame kind into a set of flags. r=njn
authorMarkus Stange <mstange@themasta.com>
Tue, 06 Nov 2018 04:32:29 +0000
changeset 444531 be8a9a7f29bda128366fd1d08097fe34d8c717e2
parent 444530 76602ccd5ca3063fbcdf1dd02f2df783265ba70e
child 444532 2bc3310f933cb414575ab76435e5e62e8d6fdb82
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 - Convert the ProfilingStackFrame kind into a set of flags. r=njn This makes it easier to add more flags. Depends on D9197 Differential Revision: https://phabricator.services.mozilla.com/D9199
js/public/ProfilingStack.h
js/src/vm/GeckoProfiler.cpp
tools/profiler/core/ProfileBuffer.cpp
tools/profiler/core/platform.cpp
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -147,58 +147,64 @@ 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...1 hold the Kind. Bits 2...31 hold the category.
+    // Bits 0...3 hold the Flags. Bits 4...31 hold the category.
     mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire,
-                    mozilla::recordreplay::Behavior::DontPreserve> kindAndCategory_;
+                    mozilla::recordreplay::Behavior::DontPreserve> flagsAndCategory_;
 
     static int32_t pcToOffset(JSScript* aScript, jsbytecode* aPc);
 
   public:
     ProfilingStackFrame() = default;
     ProfilingStackFrame& operator=(const ProfilingStackFrame& other)
     {
         label_ = other.label();
         dynamicString_ = other.dynamicString();
         void* spScript = other.spOrScript;
         spOrScript = spScript;
         int32_t offsetIfJS = other.pcOffsetIfJS_;
         pcOffsetIfJS_ = offsetIfJS;
-        uint32_t kindAndCategory = other.kindAndCategory_;
-        kindAndCategory_ = kindAndCategory;
+        uint32_t flagsAndCategory = other.flagsAndCategory_;
+        flagsAndCategory_ = flagsAndCategory;
         return *this;
     }
 
-    enum class Kind : uint32_t {
+    // 4 bits for the flags.
+    // That leaves 32 - 4 = 28 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.
-        LABEL = 0,
+        IS_LABEL_FRAME = 1 << 0,
 
         // A special frame indicating the start of a run of JS profiling stack
-        // frames. SP_MARKER frames are ignored, except for the sp field.
-        // These frames are needed to get correct ordering between JS and LABEL
-        // frames because JS frames don't carry sp information.
+        // frames. IS_SP_MARKER_FRAME frames are ignored, except for the sp
+        // field. These frames are needed to get correct ordering between JS
+        // and LABEL frames because JS frames don't carry sp information.
         // SP is short for "stack pointer".
-        SP_MARKER = 1,
+        IS_SP_MARKER_FRAME = 1 << 1,
 
-        // A normal JS frame.
-        JS_NORMAL = 2,
+        // A JS frame.
+        IS_JS_FRAME = 1 << 2,
 
-        // An interpreter JS frame that has OSR-ed into baseline. JS_NORMAL
-        // frames can be converted to JS_OSR and back. JS_OSR frames are
-        // ignored.
-        JS_OSR = 3,
+        // 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,
 
-        KIND_BITCOUNT = 2,
-        KIND_MASK = (1 << KIND_BITCOUNT) - 1
+        FLAGS_BITCOUNT = 4,
+        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,
         JS,
@@ -206,82 +212,93 @@ class ProfilingStackFrame
         NETWORK,
         GRAPHICS,
         DOM,
 
         FIRST    = OTHER,
         LAST     = DOM,
     };
 
-    static_assert(uint32_t(Category::LAST) <= (UINT32_MAX >> uint32_t(Kind::KIND_BITCOUNT)),
-                  "Too many categories to fit into u32 with two bits reserved for the kind");
+    static_assert(uint32_t(Category::LAST) <= (UINT32_MAX >> uint32_t(Flags::FLAGS_BITCOUNT)),
+                  "Too many categories to fit into u32 with together with the reserved bits for the flags");
 
     bool isLabelFrame() const
     {
-        return kind() == Kind::LABEL;
+        return uint32_t(flagsAndCategory_) & uint32_t(Flags::IS_LABEL_FRAME);
     }
 
     bool isSpMarkerFrame() const
     {
-        return kind() == Kind::SP_MARKER;
+        return uint32_t(flagsAndCategory_) & uint32_t(Flags::IS_SP_MARKER_FRAME);
     }
 
     bool isJsFrame() const
     {
-        Kind k = kind();
-        return k == Kind::JS_NORMAL || k == Kind::JS_OSR;
+        return uint32_t(flagsAndCategory_) & uint32_t(Flags::IS_JS_FRAME);
+    }
+
+    bool isOSRFrame() const {
+        return uint32_t(flagsAndCategory_) & uint32_t(Flags::JS_OSR);
+    }
+
+    void setIsOSRFrame(bool isOSR) {
+        if (isOSR) {
+            flagsAndCategory_ =
+                uint32_t(flagsAndCategory_) | uint32_t(Flags::JS_OSR);
+        } else {
+            flagsAndCategory_ =
+                uint32_t(flagsAndCategory_) & ~uint32_t(Flags::JS_OSR);
+        }
     }
 
     void setLabel(const char* aLabel) { label_ = aLabel; }
     const char* label() const { return label_; }
 
     const char* dynamicString() const { return dynamicString_; }
 
     void initLabelFrame(const char* aLabel, const char* aDynamicString, void* sp,
                         Category aCategory)
     {
         label_ = aLabel;
         dynamicString_ = aDynamicString;
         spOrScript = sp;
         // pcOffsetIfJS_ is not set and must not be used on label frames.
-        kindAndCategory_ = uint32_t(Kind::LABEL) | (uint32_t(aCategory) << uint32_t(Kind::KIND_BITCOUNT));
+        flagsAndCategory_ =
+            uint32_t(Flags::IS_LABEL_FRAME) |
+            (uint32_t(aCategory) << uint32_t(Flags::FLAGS_BITCOUNT));
         MOZ_ASSERT(isLabelFrame());
     }
 
     void initSpMarkerFrame(void* sp)
     {
         label_ = "";
         dynamicString_ = nullptr;
         spOrScript = sp;
         // pcOffsetIfJS_ is not set and must not be used on sp marker frames.
-        kindAndCategory_ = uint32_t(Kind::SP_MARKER) | (uint32_t(Category::OTHER) << uint32_t(Kind::KIND_BITCOUNT));
+        flagsAndCategory_ =
+            uint32_t(Flags::IS_SP_MARKER_FRAME) |
+            (uint32_t(Category::OTHER) << uint32_t(Flags::FLAGS_BITCOUNT));
         MOZ_ASSERT(isSpMarkerFrame());
     }
 
     void initJsFrame(const char* aLabel, const char* aDynamicString, JSScript* aScript,
                      jsbytecode* aPc)
     {
         label_ = aLabel;
         dynamicString_ = aDynamicString;
         spOrScript = aScript;
         pcOffsetIfJS_ = pcToOffset(aScript, aPc);
-        kindAndCategory_ = uint32_t(Kind::JS_NORMAL) | (uint32_t(Category::JS) << uint32_t(Kind::KIND_BITCOUNT));
+        flagsAndCategory_ =
+            uint32_t(Flags::IS_JS_FRAME) |
+            (uint32_t(Category::JS) << uint32_t(Flags::FLAGS_BITCOUNT));
         MOZ_ASSERT(isJsFrame());
     }
 
-    void setKind(Kind aKind) {
-        kindAndCategory_ = uint32_t(aKind) | (uint32_t(category()) << uint32_t(Kind::KIND_BITCOUNT));
-    }
-
-    Kind kind() const {
-        return Kind(kindAndCategory_ & uint32_t(Kind::KIND_MASK));
-    }
-
     Category category() const {
-        return Category(kindAndCategory_ >> uint32_t(Kind::KIND_BITCOUNT));
+        return Category(flagsAndCategory_ >> uint32_t(Flags::FLAGS_BITCOUNT));
     }
 
     void* stackAddress() const {
         MOZ_ASSERT(!isJsFrame());
         return spOrScript;
     }
 
     JS_PUBLIC_API(JSScript*) script() const;
--- a/js/src/vm/GeckoProfiler.cpp
+++ b/js/src/vm/GeckoProfiler.cpp
@@ -398,35 +398,35 @@ GeckoProfilerBaselineOSRMarker::GeckoPro
     }
 
     spBefore_ = sp;
     if (sp == 0) {
         return;
     }
 
     ProfilingStackFrame& frame = profiler->profilingStack_->frames[sp - 1];
-    MOZ_ASSERT(frame.kind() == ProfilingStackFrame::Kind::JS_NORMAL);
-    frame.setKind(ProfilingStackFrame::Kind::JS_OSR);
+    MOZ_ASSERT(!frame.isOSRFrame());
+    frame.setIsOSRFrame(true);
 }
 
 GeckoProfilerBaselineOSRMarker::~GeckoProfilerBaselineOSRMarker()
 {
     if (profiler == nullptr) {
         return;
     }
 
     uint32_t sp = profiler->stackPointer();
     MOZ_ASSERT(spBefore_ == sp);
     if (sp == 0) {
         return;
     }
 
     ProfilingStackFrame& frame = profiler->stack()[sp - 1];
-    MOZ_ASSERT(frame.kind() == ProfilingStackFrame::Kind::JS_OSR);
-    frame.setKind(ProfilingStackFrame::Kind::JS_NORMAL);
+    MOZ_ASSERT(frame.isOSRFrame());
+    frame.setIsOSRFrame(false);
 }
 
 JS_PUBLIC_API(JSScript*)
 ProfilingStackFrame::script() const
 {
     MOZ_ASSERT(isJsFrame());
     auto script = reinterpret_cast<JSScript*>(spOrScript.operator void*());
     if (!script) {
--- a/tools/profiler/core/ProfileBuffer.cpp
+++ b/tools/profiler/core/ProfileBuffer.cpp
@@ -157,18 +157,18 @@ ProfileBufferCollector::CollectWasmFrame
   mBuf.CollectCodeLocation("", aLabel, Nothing(), Nothing(), Nothing());
 }
 
 void
 ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame& aFrame)
 {
   // WARNING: this function runs within the profiler's "critical section".
 
-  MOZ_ASSERT(aFrame.kind() == js::ProfilingStackFrame::Kind::LABEL ||
-             aFrame.kind() == js::ProfilingStackFrame::Kind::JS_NORMAL);
+  MOZ_ASSERT(aFrame.isLabelFrame() ||
+             (aFrame.isJsFrame() && !aFrame.isOSRFrame()));
 
   const char* label = aFrame.label();
   const char* dynamicString = aFrame.dynamicString();
   bool isChromeJSEntry = false;
   Maybe<uint32_t> line;
   Maybe<uint32_t> column;
 
   if (aFrame.isJsFrame()) {
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -980,17 +980,17 @@ MergeStacks(uint32_t aFeatures, bool aIs
         lastLabelFrameStackAddr = (uint8_t*) profilingStackFrame.stackAddress();
       }
 
       // Skip any JS_OSR frames. Such frames are used when the JS interpreter
       // enters a jit frame on a loop edge (via on-stack-replacement, or OSR).
       // To avoid both the profiling stack frame and jit frame being recorded
       // (and showing up twice), the interpreter marks the interpreter
       // profiling stack frame as JS_OSR to ensure that it doesn't get counted.
-      if (profilingStackFrame.kind() == js::ProfilingStackFrame::Kind::JS_OSR) {
+      if (profilingStackFrame.isOSRFrame()) {
           profilingStackIndex++;
           continue;
       }
 
       MOZ_ASSERT(lastLabelFrameStackAddr);
       profilingStackAddr = lastLabelFrameStackAddr;
     }