Bug 1204584 - Ensure that entries created by AutoSPSEntry propogate their category information; r=djvj
authorNick Fitzgerald <fitzgen@gmail.com>
Sat, 19 Sep 2015 11:19:07 +0200
changeset 296016 f33142322571d4a84728aa03f6378ff1211c597e
parent 296015 a29828c4d7be167a1334d9de4309057e462edb56
child 296017 d52326f55d2e24bd3a700d4bf60f9ce0c16b03bd
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1204584
milestone43.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 1204584 - Ensure that entries created by AutoSPSEntry propogate their category information; r=djvj This commit renames ProfileEntry::set{Js,Cpp}Frame methods to ProfileEntry::init{Js,Cpp}Frame to highlight the fact that they are intended to initialize the entry, and that setting other flags should happen after one of these calls.
js/public/ProfilingStack.h
js/src/vm/SPSProfiler.cpp
tools/profiler/public/PseudoStack.h
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -47,17 +47,18 @@ class ProfileEntry
     // General purpose storage describing this frame.
     uint32_t volatile flags_;
 
   public:
     // These traits are bit masks. Make sure they're powers of 2.
     enum Flags {
         // 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, call `setJsFrame` or `setCppFrame`.
+        // change the frame type. Instead, initialize the ProfileEntry as either
+        // a JS or CPP frame with `initJsFrame` or `initCppFrame` respectively.
         IS_CPP_ENTRY = 0x01,
 
         // Indicate that copying the frame label is not necessary when taking a
         // sample of the pseudostack.
         FRAME_LABEL_COPY = 0x02,
 
         // This ProfileEntry is a dummy entry indicating the start of a run
         // of JS pseudostack entries.
@@ -101,22 +102,22 @@ class ProfileEntry
     bool isCpp() const volatile { return hasFlag(IS_CPP_ENTRY); }
     bool isJs() const volatile { return !isCpp(); }
 
     bool isCopyLabel() const volatile { return hasFlag(FRAME_LABEL_COPY); }
 
     void setLabel(const char* aString) volatile { string = aString; }
     const char* label() const volatile { return string; }
 
-    void setJsFrame(JSScript* aScript, jsbytecode* aPc) volatile {
+    void initJsFrame(JSScript* aScript, jsbytecode* aPc) volatile {
         flags_ = 0;
         spOrScript = aScript;
         setPC(aPc);
     }
-    void setCppFrame(void* aSp, uint32_t aLine) volatile {
+    void initCppFrame(void* aSp, uint32_t aLine) volatile {
         flags_ = IS_CPP_ENTRY;
         spOrScript = aSp;
         lineOrPc = static_cast<int32_t>(aLine);
     }
 
     void setFlag(uint32_t flag) volatile {
         MOZ_ASSERT(flag != IS_CPP_ENTRY);
         flags_ |= flag;
@@ -132,16 +133,18 @@ class ProfileEntry
     uint32_t flags() const volatile {
         return flags_;
     }
 
     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(static_cast<uint32_t>(c));
     }
 
     void setOSR() volatile {
         MOZ_ASSERT(isJs());
         setFlag(OSR);
     }
--- a/js/src/vm/SPSProfiler.cpp
+++ b/js/src/vm/SPSProfiler.cpp
@@ -248,17 +248,17 @@ SPSProfiler::beginPseudoJS(const char* s
     /* these operations cannot be re-ordered, so volatile-ize operations */
     volatile ProfileEntry* stack = stack_;
     volatile uint32_t* size = size_;
     uint32_t current = *size;
 
     MOZ_ASSERT(installed());
     if (current < max_) {
         stack[current].setLabel(string);
-        stack[current].setCppFrame(sp, 0);
+        stack[current].initCppFrame(sp, 0);
         stack[current].setFlag(ProfileEntry::BEGIN_PSEUDO_JS);
     }
     *size = current + 1;
 }
 
 void
 SPSProfiler::push(const char* string, void* sp, JSScript* script, jsbytecode* pc, bool copy,
                   ProfileEntry::Category category)
@@ -269,28 +269,29 @@ SPSProfiler::push(const char* string, vo
     /* these operations cannot be re-ordered, so volatile-ize operations */
     volatile ProfileEntry* stack = stack_;
     volatile uint32_t* size = size_;
     uint32_t current = *size;
 
     MOZ_ASSERT(installed());
     if (current < max_) {
         volatile ProfileEntry& entry = stack[current];
-        entry.setLabel(string);
-        entry.setCategory(category);
 
         if (sp != nullptr) {
-            entry.setCppFrame(sp, 0);
+            entry.initCppFrame(sp, 0);
             MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY);
         }
         else {
-            entry.setJsFrame(script, pc);
+            entry.initJsFrame(script, pc);
             MOZ_ASSERT(entry.flags() == 0);
         }
 
+        entry.setLabel(string);
+        entry.setCategory(category);
+
         // Track if mLabel needs a copy.
         if (copy)
             entry.setFlag(js::ProfileEntry::FRAME_LABEL_COPY);
         else
             entry.unsetFlag(js::ProfileEntry::FRAME_LABEL_COPY);
     }
     *size = current + 1;
 }
--- a/tools/profiler/public/PseudoStack.h
+++ b/tools/profiler/public/PseudoStack.h
@@ -271,26 +271,20 @@ public:
     if (mStackPointer == 0) {
       ref();
     }
 
     volatile StackEntry &entry = mStack[mStackPointer];
 
     // Make sure we increment the pointer after the name has
     // been written such that mStack is always consistent.
+    entry.initCppFrame(aStackAddress, line);
     entry.setLabel(aName);
-    entry.setCppFrame(aStackAddress, line);
     MOZ_ASSERT(entry.flags() == js::ProfileEntry::IS_CPP_ENTRY);
-
-    uint32_t uint_category = static_cast<uint32_t>(aCategory);
-    MOZ_ASSERT(
-      uint_category >= static_cast<uint32_t>(js::ProfileEntry::Category::FIRST) &&
-      uint_category <= static_cast<uint32_t>(js::ProfileEntry::Category::LAST));
-
-    entry.setFlag(uint_category);
+    entry.setCategory(aCategory);
 
     // Track if mLabel needs a copy.
     if (aCopy)
       entry.setFlag(js::ProfileEntry::FRAME_LABEL_COPY);
     else
       entry.unsetFlag(js::ProfileEntry::FRAME_LABEL_COPY);
 
     // Prevent the optimizer from re-ordering these instructions
@@ -461,9 +455,8 @@ public:
     int newValue = --mRefCnt;
     if (newValue == 0) {
       delete this;
     }
   }
 };
 
 #endif
-