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

--HG--
extra : moz-landing-system : lando

js/public/ProfilingStack.h

@@ -152,9 +152,9 @@ class ProfilingStackFrame
     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);
 
@@ -168,32 +168,38 @@ class ProfilingStackFrame
         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
@@ -211,23 +217,36 @@ class ProfilingStackFrame
         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; }
@@ -242,7 +261,9 @@ class ProfilingStackFrame
         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());
     }
 
@@ -252,7 +273,9 @@ class ProfilingStackFrame
         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());
     }
 
@@ -263,20 +286,14 @@ class ProfilingStackFrame
         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 {

js/src/vm/GeckoProfiler.cpp

@@ -403,8 +403,8 @@ GeckoProfilerBaselineOSRMarker::GeckoProfilerBaselineOSRMarker(JSContext* cx, bo
     }
 
     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()
@@ -420,8 +420,8 @@ GeckoProfilerBaselineOSRMarker::~GeckoProfilerBaselineOSRMarker()
     }
 
     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*)

tools/profiler/core/ProfileBuffer.cpp

@@ -162,8 +162,8 @@ ProfileBufferCollector::CollectProfilingStackFrame(const js::ProfilingStackFrame
 {
   // 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();

tools/profiler/core/platform.cpp

@@ -985,7 +985,7 @@ MergeStacks(uint32_t aFeatures, bool aIsSynchronous,
       // 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;
       }