From e6c22c99ad343b9adcdc5db6dec82890f9afb7ac Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 6 Nov 2018 04:32:29 +0000 Subject: [PATCH] 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 | 89 ++++++++++++++++----------- js/src/vm/GeckoProfiler.cpp | 8 +-- tools/profiler/core/ProfileBuffer.cpp | 4 +- tools/profiler/core/platform.cpp | 2 +- 4 files changed, 60 insertions(+), 43 deletions(-) diff --git a/js/public/ProfilingStack.h b/js/public/ProfilingStack.h index a10210535877..a4601b6e552a 100644 --- a/js/public/ProfilingStack.h +++ b/js/public/ProfilingStack.h @@ -152,9 +152,9 @@ class ProfilingStackFrame mozilla::Atomic 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 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 { diff --git a/js/src/vm/GeckoProfiler.cpp b/js/src/vm/GeckoProfiler.cpp index 6568f5e4cce5..14091b166f8b 100644 --- a/js/src/vm/GeckoProfiler.cpp +++ b/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*) diff --git a/tools/profiler/core/ProfileBuffer.cpp b/tools/profiler/core/ProfileBuffer.cpp index fe143732ed9e..5d5525e1a31e 100644 --- a/tools/profiler/core/ProfileBuffer.cpp +++ b/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(); diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index f5e5be2b226e..3cac2affa78f 100644 --- a/tools/profiler/core/platform.cpp +++ b/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; }