Bug 1437428 - Make PseudoStack a member of RacyInfo instead of inheriting from it. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Wed, 31 Jan 2018 17:42:49 -0500
changeset 756426 7566ff3df164ffd81173b446ac5b7bb39040df2f
parent 756422 c4d818c138689a66a397393fe1dd259ef0943b35
child 756427 26ca21c3a521604a7c8f7cb605140409e1861ee9
child 756635 af6c4ba0c376b0d24c231ac9bf14985250913bf5
push id99486
push userbmo:mstange@themasta.com
push dateFri, 16 Feb 2018 23:26:36 +0000
reviewersnjn
bugs1437428
milestone60.0a1
Bug 1437428 - Make PseudoStack a member of RacyInfo instead of inheriting from it. r?njn MozReview-Commit-ID: 3fumT1Livf6
js/public/ProfilingStack.h
tools/profiler/core/ThreadInfo.h
tools/profiler/core/platform.cpp
--- a/js/public/ProfilingStack.h
+++ b/js/public/ProfilingStack.h
@@ -289,17 +289,17 @@ RegisterContextProfilingEventMarker(JSCo
 //   entry visible to the sampler thread -- only after the new entry has been
 //   fully written. The stack pointer is Atomic<uint32_t,ReleaseAcquire>, so
 //   the increment is a release-store, which ensures that this store is not
 //   reordered before the writes of the entry.
 //
 // - When popping an old entry, the only operation is the decrementing of the
 //   stack pointer, which is obviously atomic.
 //
-class PseudoStack
+class PseudoStack final
 {
   public:
     PseudoStack()
       : stackPointer(0)
     {}
 
     ~PseudoStack() {
         // The label macros keep a reference to the PseudoStack to avoid a TLS
--- a/tools/profiler/core/ThreadInfo.h
+++ b/tools/profiler/core/ThreadInfo.h
@@ -15,22 +15,21 @@
 #include "ProfileBuffer.h"
 #include "js/ProfilingStack.h"
 
 // This class contains the info for a single thread that is accessible without
 // protection from gPSMutex in platform.cpp. Because there is no external
 // protection against data races, it must provide internal protection. Hence
 // the "Racy" prefix.
 //
-class RacyThreadInfo final : public PseudoStack
+class RacyThreadInfo final
 {
 public:
   explicit RacyThreadInfo(int aThreadId)
-    : PseudoStack()
-    , mThreadId(aThreadId)
+    : mThreadId(aThreadId)
     , mSleep(AWAKE)
   {
     MOZ_COUNT_CTOR(RacyThreadInfo);
   }
 
   ~RacyThreadInfo()
   {
     MOZ_COUNT_DTOR(RacyThreadInfo);
@@ -111,17 +110,21 @@ public:
     MOZ_ASSERT(mSleep != AWAKE);
     mSleep = AWAKE;
   }
 
   bool IsSleeping() { return mSleep != AWAKE; }
 
   int ThreadId() const { return mThreadId; }
 
+  class PseudoStack& PseudoStack() { return mPseudoStack; }
+
 private:
+  class PseudoStack mPseudoStack;
+
   // A list of pending markers that must be moved to the circular buffer.
   ProfilerSignalSafeLinkedList<ProfilerMarker> mPendingMarkers;
 
   // mThreadId contains the thread ID of the current thread. It is safe to read
   // this from multiple threads concurrently, as it will never be mutated.
   const int mThreadId;
 
   // mSleep tracks whether the thread is sleeping, and if so, whether it has
@@ -270,20 +273,19 @@ public:
   void SetJSContext(JSContext* aContext)
   {
     // This function runs on-thread.
 
     MOZ_ASSERT(aContext && !mContext);
 
     mContext = aContext;
 
-    // We give the JS engine a non-owning reference to the RacyInfo (just the
-    // PseudoStack, really). It's important that the JS engine doesn't touch
-    // this once the thread dies.
-    js::SetContextProfilingStack(aContext, RacyInfo());
+    // We give the JS engine a non-owning reference to the PseudoStack. It's
+    // important that the JS engine doesn't touch this once the thread dies.
+    js::SetContextProfilingStack(aContext, &RacyInfo()->PseudoStack());
 
     PollJSSampling();
   }
 
   // Request that this thread start JS sampling. JS sampling won't actually
   // start until a subsequent PollJSSampling() call occurs *and* mContext has
   // been set.
   void StartJSSampling()
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -629,17 +629,17 @@ public:
   // can also be used to get the PseudoStack, but that is marginally slower
   // because it requires an extra pointer indirection.
   static PseudoStack* Stack() { return AutoProfilerLabel::sPseudoStack.get(); }
 
   static void SetInfo(PSLockRef, ThreadInfo* aInfo)
   {
     sThreadInfo.set(aInfo);
     AutoProfilerLabel::sPseudoStack.set(
-      aInfo ? aInfo->RacyInfo().get() : nullptr);  // an upcast
+      aInfo ? &aInfo->RacyInfo()->PseudoStack() : nullptr);
   }
 
 private:
   // This is a non-owning reference to the ThreadInfo; CorePS::mLiveThreads is
   // the owning reference. On thread destruction, this reference is cleared and
   // the ThreadInfo is destroyed or transferred to CorePS::mDeadThreads.
   static MOZ_THREAD_LOCAL(ThreadInfo*) sThreadInfo;
 };
@@ -737,19 +737,19 @@ MergeStacks(uint32_t aFeatures, bool aIs
             const ThreadInfo& aThreadInfo, const Registers& aRegs,
             const NativeStack& aNativeStack,
             ProfilerStackCollector& aCollector)
 {
   // WARNING: this function runs within the profiler's "critical section".
   // WARNING: this function might be called while the profiler is inactive, and
   //          cannot rely on ActivePS.
 
-  NotNull<RacyThreadInfo*> racyInfo = aThreadInfo.RacyInfo();
-  js::ProfileEntry* pseudoEntries = racyInfo->entries;
-  uint32_t pseudoCount = racyInfo->stackSize();
+  PseudoStack& pseudoStack = aThreadInfo.RacyInfo()->PseudoStack();
+  js::ProfileEntry* pseudoEntries = pseudoStack.entries;
+  uint32_t pseudoCount = pseudoStack.stackSize();
   JSContext* context = aThreadInfo.mContext;
 
   // Make a copy of the JS stack into a JSFrame array. This is necessary since,
   // like the native stack, the JS stack is iterated youngest-to-oldest and we
   // need to iterate oldest-to-youngest when adding entries to aInfo.
 
   // Non-periodic sampling passes Nothing() as the buffer write position to
   // ProfilingFrameIterator to avoid incorrectly resetting the buffer position
@@ -867,17 +867,17 @@ MergeStacks(uint32_t aFeatures, bool aIs
       MOZ_ASSERT(pseudoIndex < pseudoCount);
       js::ProfileEntry& pseudoEntry = pseudoEntries[pseudoIndex];
 
       // Pseudo-frames with the CPP_MARKER_FOR_JS kind are just annotations and
       // should not be recorded in the profile.
       if (pseudoEntry.kind() != js::ProfileEntry::Kind::CPP_MARKER_FOR_JS) {
         // The JIT only allows the top-most entry to have a nullptr pc.
         MOZ_ASSERT_IF(pseudoEntry.isJs() && pseudoEntry.script() && !pseudoEntry.pc(),
-                      &pseudoEntry == &racyInfo->entries[racyInfo->stackSize() - 1]);
+                      &pseudoEntry == &pseudoStack.entries[pseudoStack.stackSize() - 1]);
         aCollector.CollectPseudoEntry(pseudoEntry);
       }
       pseudoIndex++;
       continue;
     }
 
     // Check to see if JS jit stack frame is top-most
     if (jsStackAddr > nativeStackAddr) {
@@ -1006,26 +1006,26 @@ DoEHABIBacktrace(PSLockRef aLock, const 
                  const Registers& aRegs, NativeStack& aNativeStack)
 {
   // WARNING: this function runs within the profiler's "critical section".
   // WARNING: this function might be called while the profiler is inactive, and
   //          cannot rely on ActivePS.
 
   const mcontext_t* mcontext = &aRegs.mContext->uc_mcontext;
   mcontext_t savedContext;
-  NotNull<RacyThreadInfo*> racyInfo = aThreadInfo.RacyInfo();
+  PseudoStack& pseudoStack = aThreadInfo.RacyInfo()->PseudoStack();
 
   // The pseudostack contains an "EnterJIT" frame whenever we enter
   // JIT code with profiling enabled; the stack pointer value points
   // the saved registers.  We use this to unwind resume unwinding
   // after encounting JIT code.
-  for (uint32_t i = racyInfo->stackSize(); i > 0; --i) {
+  for (uint32_t i = pseudoStack.stackSize(); i > 0; --i) {
     // The pseudostack grows towards higher indices, so we iterate
     // backwards (from callee to caller).
-    js::ProfileEntry& entry = racyInfo->entries[i - 1];
+    js::ProfileEntry& entry = pseudoStack.entries[i - 1];
     if (!entry.isJs() && strcmp(entry.label(), "EnterJIT") == 0) {
       // Found JIT entry frame.  Unwind up to that point (i.e., force
       // the stack walk to stop before the block of saved registers;
       // note that it yields nondecreasing stack pointers), then restore
       // the saved state.
       uint32_t* vSP = reinterpret_cast<uint32_t*>(entry.stackAddress());
 
       aNativeStack.mCount +=