Bug 1367654 (part 1) - Clean up ProfileEntry. r=shu.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 26 May 2017 14:54:31 +1000
changeset 409579 7b82e5e570d6e28138167115fe74a2d5d32c9e07
parent 409578 e4ac809a2e76161c335b9a8dbac2a4a243f33952
child 409580 343d001b8dfe9746d5423d0d1fca61c248805cc9
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1367654
milestone55.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 1367654 (part 1) - Clean up ProfileEntry. r=shu. This patch: - renames flags_ as flagsAndCategory_ because it contains both the flags and the category; - uses << for some bitfield definitions, because it's easier to read; - removes some dead methods from ProfileEntry; - removes the unnecessary JS_FRIEND_API from setPC().
js/public/ProfilingStack.h
js/src/vm/GeckoProfiler.cpp
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -50,56 +50,56 @@ class ProfileEntry
     const char * volatile dynamicString_;
 
     // Stack pointer for non-JS entries, the script pointer otherwise.
     void * volatile spOrScript;
 
     // Line number for non-JS entries, the bytecode offset otherwise.
     int32_t volatile lineOrPcOffset;
 
-    // General purpose storage describing this frame.
-    uint32_t volatile flags_;
+    // Flags are in the low bits. The category is in the high bits.
+    uint32_t volatile flagsAndCategory_;
 
     static int32_t pcToOffset(JSScript* aScript, jsbytecode* aPc);
 
   public:
     // These traits are bit masks. Make sure they're powers of 2.
     enum Flags : uint32_t {
         // Indicate whether a profile entry represents a CPP frame. If not set,
         // a JS frame is assumed by default. You're not allowed to publicly
         // change the frame type. Instead, initialize the ProfileEntry as either
         // a JS or CPP frame with `initJsFrame` or `initCppFrame` respectively.
-        IS_CPP_ENTRY = 1 << 0,
+        IS_CPP_ENTRY = 1u << 0,
 
         // This ProfileEntry is a dummy entry indicating the start of a run
         // of JS pseudostack entries.
-        BEGIN_PSEUDO_JS = 1 << 1,
+        BEGIN_PSEUDO_JS = 1u << 1,
 
         // This flag is used to indicate that an interpreter JS entry has OSR-ed
         // into baseline.
-        OSR = 1 << 2,
+        OSR = 1u << 2,
 
         // Union of all flags.
         ALL = IS_CPP_ENTRY|BEGIN_PSEUDO_JS|OSR,
 
         // Mask for removing all flags except the category information.
         CATEGORY_MASK = ~ALL
     };
 
     // Keep these in sync with devtools/client/performance/modules/categories.js
     enum class Category : uint32_t {
-        OTHER    = 0x10,
-        CSS      = 0x20,
-        JS       = 0x40,
-        GC       = 0x80,
-        CC       = 0x100,
-        NETWORK  = 0x200,
-        GRAPHICS = 0x400,
-        STORAGE  = 0x800,
-        EVENTS   = 0x1000,
+        OTHER    = 1u << 4,
+        CSS      = 1u << 5,
+        JS       = 1u << 6,
+        GC       = 1u << 7,
+        CC       = 1u << 8,
+        NETWORK  = 1u << 9,
+        GRAPHICS = 1u << 10,
+        STORAGE  = 1u << 11,
+        EVENTS   = 1u << 12,
 
         FIRST    = OTHER,
         LAST     = EVENTS
     };
 
     static_assert((static_cast<int>(Category::FIRST) & Flags::ALL) == 0,
                   "The category bitflags should not intersect with the other flags!");
 
@@ -119,53 +119,43 @@ class ProfileEntry
     void initCppFrame(const char* aLabel, const char* aDynamicString, void* sp, uint32_t aLine,
                       js::ProfileEntry::Flags aFlags, js::ProfileEntry::Category aCategory)
                       volatile
     {
         label_ = aLabel;
         dynamicString_ = aDynamicString;
         spOrScript = sp;
         lineOrPcOffset = static_cast<int32_t>(aLine);
-        flags_ = aFlags | js::ProfileEntry::IS_CPP_ENTRY | uint32_t(aCategory);
+        flagsAndCategory_ = aFlags | js::ProfileEntry::IS_CPP_ENTRY | uint32_t(aCategory);
     }
 
     void initJsFrame(const char* aLabel, const char* aDynamicString, JSScript* aScript,
                      jsbytecode* aPc) volatile
     {
         label_ = aLabel;
         dynamicString_ = aDynamicString;
         spOrScript = aScript;
         lineOrPcOffset = pcToOffset(aScript, aPc);
-        flags_ = uint32_t(js::ProfileEntry::Category::JS);  // No flags, just the JS category.
+        flagsAndCategory_ = uint32_t(js::ProfileEntry::Category::JS);  // No flags needed.
     }
 
     void setFlag(uint32_t flag) volatile {
         MOZ_ASSERT(flag != IS_CPP_ENTRY);
-        flags_ |= flag;
+        flagsAndCategory_ |= flag;
     }
     void unsetFlag(uint32_t flag) volatile {
         MOZ_ASSERT(flag != IS_CPP_ENTRY);
-        flags_ &= ~flag;
+        flagsAndCategory_ &= ~flag;
     }
     bool hasFlag(uint32_t flag) const volatile {
-        return bool(flags_ & flag);
-    }
-
-    uint32_t flags() const volatile {
-        return flags_;
+        return bool(flagsAndCategory_ & flag);
     }
 
     uint32_t category() const volatile {
-        return flags_ & CATEGORY_MASK;
-    }
-    void setCategory(Category c) volatile {
-        MOZ_ASSERT(c >= Category::FIRST);
-        MOZ_ASSERT(c <= Category::LAST);
-        flags_ &= ~CATEGORY_MASK;
-        setFlag(uint32_t(c));
+        return flagsAndCategory_ & CATEGORY_MASK;
     }
 
     void setOSR() volatile {
         MOZ_ASSERT(isJs());
         setFlag(OSR);
     }
     void unsetOSR() volatile {
         MOZ_ASSERT(isJs());
@@ -174,43 +164,40 @@ class ProfileEntry
     bool isOSR() const volatile {
         return hasFlag(OSR);
     }
 
     void* stackAddress() const volatile {
         MOZ_ASSERT(!isJs());
         return spOrScript;
     }
+
     JS_PUBLIC_API(JSScript*) script() const volatile;
+
     uint32_t line() const volatile {
         MOZ_ASSERT(!isJs());
         return static_cast<uint32_t>(lineOrPcOffset);
     }
 
     // Note that the pointer returned might be invalid.
     JSScript* rawScript() const volatile {
         MOZ_ASSERT(isJs());
         return (JSScript*)spOrScript;
     }
 
     // We can't know the layout of JSScript, so look in vm/GeckoProfiler.cpp.
     JS_FRIEND_API(jsbytecode*) pc() const volatile;
-    JS_FRIEND_API(void) setPC(jsbytecode* pc) volatile;
+    void setPC(jsbytecode* pc) volatile;
 
     void trace(JSTracer* trc) volatile;
 
     // The offset of a pc into a script's code can actually be 0, so to
     // signify a nullptr pc, use a -1 index. This is checked against in
     // pc() and setPC() to set/get the right pc.
     static const int32_t NullPCOffset = -1;
-
-    static size_t offsetOfLabel() { return offsetof(ProfileEntry, label_); }
-    static size_t offsetOfSpOrScript() { return offsetof(ProfileEntry, spOrScript); }
-    static size_t offsetOfLineOrPcOffset() { return offsetof(ProfileEntry, lineOrPcOffset); }
-    static size_t offsetOfFlags() { return offsetof(ProfileEntry, flags_); }
 };
 
 JS_FRIEND_API(void)
 SetContextProfilingStack(JSContext* cx, PseudoStack* pseudoStack);
 
 JS_FRIEND_API(void)
 EnableContextProfilingStack(JSContext* cx, bool enabled);
 
--- a/js/src/vm/GeckoProfiler.cpp
+++ b/js/src/vm/GeckoProfiler.cpp
@@ -497,17 +497,17 @@ ProfileEntry::pc() const volatile
     return script ? script->offsetToPC(lineOrPcOffset) : nullptr;
 }
 
 /* static */ int32_t
 ProfileEntry::pcToOffset(JSScript* aScript, jsbytecode* aPc) {
     return aPc ? aScript->pcToOffset(aPc) : NullPCOffset;
 }
 
-JS_FRIEND_API(void)
+void
 ProfileEntry::setPC(jsbytecode* pc) volatile
 {
     MOZ_ASSERT(isJs());
     JSScript* script = this->script();
     MOZ_ASSERT(script); // This should not be called while profiling is suppressed.
     lineOrPcOffset = pcToOffset(script, pc);
 }