Bug 1434965 - Remove OnStackFrameKey and solve frame canonicalization differently. r?njn draft
authorMarkus Stange <mstange@themasta.com>
Mon, 12 Feb 2018 14:00:05 -0500
changeset 754059 6891c97cdeb19401891030ea852eb8215d6d11fb
parent 754058 b8094547943730f7d3c1e582a3f13dbcb2ecabce
child 754060 efea50db02a1accb1949266a04f50750603827dd
push id98744
push userbmo:mstange@themasta.com
push dateMon, 12 Feb 2018 19:50:57 +0000
reviewersnjn
bugs1434965
milestone60.0a1
Bug 1434965 - Remove OnStackFrameKey and solve frame canonicalization differently. r?njn MozReview-Commit-ID: DITzwF94U64
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ProfileBufferEntry.h
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -272,23 +272,23 @@ bool UniqueStacks::FrameKey::operator==(
   return mLocation == aOther.mLocation &&
          mLine == aOther.mLine &&
          mCategory == aOther.mCategory &&
          mJITAddress == aOther.mJITAddress &&
          mJITDepth == aOther.mJITDepth;
 }
 
 UniqueStacks::StackKey
-UniqueStacks::BeginStack(const OnStackFrameKey& aFrame)
+UniqueStacks::BeginStack(const FrameKey& aFrame)
 {
   return StackKey(GetOrAddFrameIndex(aFrame));
 }
 
 UniqueStacks::StackKey
-UniqueStacks::AppendFrame(const StackKey& aStack, const OnStackFrameKey& aFrame)
+UniqueStacks::AppendFrame(const StackKey& aStack, const FrameKey& aFrame)
 {
   return StackKey(aStack, GetOrAddStackIndex(aStack), GetOrAddFrameIndex(aFrame));
 }
 
 uint32_t UniqueStacks::FrameKey::Hash() const
 {
   uint32_t hash = 0;
   if (!mLocation.IsEmpty()) {
@@ -306,17 +306,16 @@ uint32_t UniqueStacks::FrameKey::Hash() 
       hash = mozilla::AddToHash(hash, *mJITDepth);
     }
   }
   return hash;
 }
 
 UniqueStacks::UniqueStacks(JSContext* aContext)
  : mContext(aContext)
- , mFrameCount(0)
 {
   mFrameTableWriter.StartBareList();
   mStackTableWriter.StartBareList();
 }
 
 uint32_t UniqueStacks::GetOrAddStackIndex(const StackKey& aStack)
 {
   uint32_t index;
@@ -326,64 +325,69 @@ uint32_t UniqueStacks::GetOrAddStackInde
   }
 
   index = mStackToIndexMap.Count();
   mStackToIndexMap.Put(aStack, index);
   StreamStack(aStack);
   return index;
 }
 
-uint32_t UniqueStacks::GetOrAddFrameIndex(const OnStackFrameKey& aFrame)
+MOZ_MUST_USE nsTArray<UniqueStacks::FrameKey>
+UniqueStacks::GetOrAddJITFrameKeysForAddress(void* aJITAddress)
+{
+  nsTArray<FrameKey>& frameKeys =
+    *mAddressToJITFrameKeysMap.LookupOrAdd(aJITAddress);
+
+  if (frameKeys.IsEmpty()) {
+    MOZ_RELEASE_ASSERT(mContext);
+    for (JS::ProfiledFrameHandle handle : JS::GetProfiledFrames(mContext,
+                                                                aJITAddress)) {
+      // JIT frames with the same canonical address should be treated as the
+      // same frame, so set the frame key's address to the canonical address.
+      FrameKey frameKey(handle.canonicalAddress(), frameKeys.Length());
+      MaybeAddJITFrameIndex(frameKey, handle);
+      frameKeys.AppendElement(frameKey);
+    }
+    MOZ_ASSERT(frameKeys.Length() > 0);
+  }
+
+  // Return a copy of the array.
+  return nsTArray<FrameKey>(frameKeys);
+}
+
+void
+UniqueStacks::MaybeAddJITFrameIndex(const FrameKey& aFrame,
+                                    const JS::ProfiledFrameHandle& aJITFrame)
 {
   uint32_t index;
   if (mFrameToIndexMap.Get(aFrame, &index)) {
-    MOZ_ASSERT(index < mFrameCount);
+    MOZ_ASSERT(index < mFrameToIndexMap.Count());
+    return;
+  }
+
+  index = mFrameToIndexMap.Count();
+  mFrameToIndexMap.Put(aFrame, index);
+  StreamJITFrame(aJITFrame);
+}
+
+uint32_t
+UniqueStacks::GetOrAddFrameIndex(const FrameKey& aFrame)
+{
+  uint32_t index;
+  if (mFrameToIndexMap.Get(aFrame, &index)) {
+    MOZ_ASSERT(index < mFrameToIndexMap.Count());
     return index;
   }
 
-  // If aFrame isn't canonical, forward it to the canonical frame's index.
-  if (aFrame.mJITFrameHandle) {
-    void* canonicalAddr = aFrame.mJITFrameHandle->canonicalAddress();
-    if (canonicalAddr != *aFrame.mJITAddress) {
-      OnStackFrameKey canonicalKey(canonicalAddr, *aFrame.mJITDepth, *aFrame.mJITFrameHandle);
-      uint32_t canonicalIndex = GetOrAddFrameIndex(canonicalKey);
-      mFrameToIndexMap.Put(aFrame, canonicalIndex);
-      return canonicalIndex;
-    }
-    // A manual count is used instead of mFrameToIndexMap.Count() due to
-    // forwarding of canonical JIT frames above.
-    index = mFrameCount++;
-    mFrameToIndexMap.Put(aFrame, index);
-    StreamJITFrame(*aFrame.mJITFrameHandle);
-  } else {
-    index = mFrameCount++;
-    mFrameToIndexMap.Put(aFrame, index);
-    StreamNonJITFrame(aFrame);
-  }
+  index = mFrameToIndexMap.Count();
+  mFrameToIndexMap.Put(aFrame, index);
+  StreamNonJITFrame(aFrame);
   return index;
 }
 
-uint32_t UniqueStacks::LookupJITFrameDepth(void* aAddr)
-{
-  uint32_t depth;
-
-  auto it = mJITFrameDepthMap.find(aAddr);
-  if (it != mJITFrameDepthMap.end()) {
-    depth = it->second;
-    MOZ_ASSERT(depth > 0);
-    return depth;
-  }
-  return 0;
-}
-
-void UniqueStacks::AddJITFrameDepth(void* aAddr, unsigned depth)
-{
-  mJITFrameDepthMap[aAddr] = depth;
-}
-
 void UniqueStacks::SpliceFrameTableElements(SpliceableJSONWriter& aWriter)
 {
   mFrameTableWriter.EndBareList();
   aWriter.TakeAndSplice(mFrameTableWriter.WriteFunc());
 }
 
 void UniqueStacks::SpliceStackTableElements(SpliceableJSONWriter& aWriter)
 {
@@ -736,29 +740,29 @@ ProfileBuffer::StreamSamplesToJSON(Splic
       if (sample.mTime < aSinceTime) {
         continue;
       }
     } else {
       ERROR_AND_CONTINUE("expected a Time entry");
     }
 
     UniqueStacks::StackKey stack =
-      aUniqueStacks.BeginStack(UniqueStacks::OnStackFrameKey("(root)"));
+      aUniqueStacks.BeginStack(UniqueStacks::FrameKey("(root)"));
 
     int numFrames = 0;
     while (e.Has()) {
       if (e.Get().IsNativeLeafAddr()) {
         numFrames++;
 
         // Bug 753041: We need a double cast here to tell GCC that we don't
         // want to sign extend 32-bit addresses starting with 0xFXXXXXX.
         unsigned long long pc = (unsigned long long)(uintptr_t)e.Get().u.mPtr;
         char buf[20];
         SprintfLiteral(buf, "%#llx", pc);
-        stack = aUniqueStacks.AppendFrame(stack, UniqueStacks::OnStackFrameKey(buf));
+        stack = aUniqueStacks.AppendFrame(stack, UniqueStacks::FrameKey(buf));
         e.Next();
 
       } else if (e.Get().IsLabel()) {
         numFrames++;
 
         // Copy the label into strbuf.
         const char* label = e.Get().u.mString;
         strncpy(strbuf.get(), label, kMaxFrameKeyLength);
@@ -788,17 +792,17 @@ ProfileBuffer::StreamSamplesToJSON(Splic
             }
             e.Next();
           } else {
             break;
           }
         }
         strbuf[kMaxFrameKeyLength - 1] = '\0';
 
-        UniqueStacks::OnStackFrameKey frameKey(strbuf.get());
+        UniqueStacks::FrameKey frameKey(strbuf.get());
 
         if (e.Has() && e.Get().IsLineNumber()) {
           frameKey.mLine = Some(unsigned(e.Get().u.mInt));
           e.Next();
         }
 
         if (e.Has() && e.Get().IsCategory()) {
           frameKey.mCategory = Some(unsigned(e.Get().u.mInt));
@@ -807,31 +811,20 @@ ProfileBuffer::StreamSamplesToJSON(Splic
 
         stack = aUniqueStacks.AppendFrame(stack, frameKey);
 
       } else if (e.Get().IsJitReturnAddr()) {
         numFrames++;
 
         // A JIT frame may expand to multiple frames due to inlining.
         void* pc = e.Get().u.mPtr;
-        unsigned depth = aUniqueStacks.LookupJITFrameDepth(pc);
-        if (depth == 0) {
-          MOZ_RELEASE_ASSERT(aContext);
-          for (JS::ProfiledFrameHandle handle : JS::GetProfiledFrames(aContext, pc)) {
-            UniqueStacks::OnStackFrameKey frameKey(pc, depth, handle);
-            stack = aUniqueStacks.AppendFrame(stack, frameKey);
-            depth++;
-          }
-          MOZ_ASSERT(depth > 0);
-          aUniqueStacks.AddJITFrameDepth(pc, depth);
-        } else {
-          for (unsigned i = 0; i < depth; i++) {
-            UniqueStacks::OnStackFrameKey inlineFrameKey(pc, i);
-            stack = aUniqueStacks.AppendFrame(stack, inlineFrameKey);
-          }
+        nsTArray<UniqueStacks::FrameKey> frameKeys =
+          aUniqueStacks.GetOrAddJITFrameKeysForAddress(pc);
+        for (const UniqueStacks::FrameKey& frameKey : frameKeys) {
+          stack = aUniqueStacks.AppendFrame(stack, frameKey);
         }
 
         e.Next();
 
       } else {
         break;
       }
     }
--- a/tools/profiler/core/ProfileBufferEntry.h
+++ b/tools/profiler/core/ProfileBufferEntry.h
@@ -19,16 +19,17 @@
 #include "js/TrackedOptimizationInfo.h"
 #include "nsHashKeys.h"
 #include "nsDataHashtable.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Vector.h"
 #include "gtest/MozGtestFriend.h"
 #include "mozilla/HashFunctions.h"
 #include "mozilla/UniquePtr.h"
+#include "nsClassHashtable.h"
 
 class ProfilerMarker;
 
 #define FOR_EACH_PROFILE_BUFFER_ENTRY_KIND(macro) \
   macro(Category,              int) \
   macro(CollectionStart,       double) \
   macro(CollectionEnd,         double) \
   macro(Label,                 const char*) \
@@ -211,42 +212,16 @@ public:
     bool operator<(const FrameKey& aOther) const {
       return mHash < aOther.mHash;
     }
 
   private:
     uint32_t mHash;
   };
 
-  // A FrameKey that holds a scoped reference to a JIT FrameHandle.
-  struct MOZ_STACK_CLASS OnStackFrameKey : public FrameKey {
-    explicit OnStackFrameKey(const char* aLocation)
-      : FrameKey(aLocation)
-      , mJITFrameHandle(nullptr)
-    { }
-
-    OnStackFrameKey(const OnStackFrameKey& aToCopy)
-      : FrameKey(aToCopy)
-      , mJITFrameHandle(aToCopy.mJITFrameHandle)
-    { }
-
-    const JS::ProfiledFrameHandle* mJITFrameHandle;
-
-    OnStackFrameKey(void* aJITAddress, unsigned aJITDepth)
-      : FrameKey(aJITAddress, aJITDepth)
-      , mJITFrameHandle(nullptr)
-    { }
-
-    OnStackFrameKey(void* aJITAddress, unsigned aJITDepth,
-                    const JS::ProfiledFrameHandle& aJITFrameHandle)
-      : FrameKey(aJITAddress, aJITDepth)
-      , mJITFrameHandle(&aJITFrameHandle)
-    { }
-  };
-
   struct StackKey {
     mozilla::Maybe<uint32_t> mPrefixStackIndex;
     uint32_t mFrameIndex;
 
     explicit StackKey(uint32_t aFrame)
      : mFrameIndex(aFrame)
      , mHash(mozilla::HashGeneric(aFrame))
     {}
@@ -271,53 +246,56 @@ public:
 
   private:
     uint32_t mHash;
   };
 
   explicit UniqueStacks(JSContext* aContext);
 
   // Return a StackKey for aFrame as the stack's root frame (no prefix).
-  MOZ_MUST_USE StackKey BeginStack(const OnStackFrameKey& aFrame);
+  MOZ_MUST_USE StackKey BeginStack(const FrameKey& aFrame);
 
   // Return a new StackKey that is obtained by appending aFrame to aStack.
   MOZ_MUST_USE StackKey AppendFrame(const StackKey& aStack,
-                                    const OnStackFrameKey& aFrame);
+                                    const FrameKey& aFrame);
 
-  MOZ_MUST_USE uint32_t GetOrAddFrameIndex(const OnStackFrameKey& aFrame);
+  MOZ_MUST_USE nsTArray<FrameKey>
+  GetOrAddJITFrameKeysForAddress(void* aJITAddress);
+
+  MOZ_MUST_USE uint32_t GetOrAddFrameIndex(const FrameKey& aFrame);
   MOZ_MUST_USE uint32_t GetOrAddStackIndex(const StackKey& aStack);
 
-  uint32_t LookupJITFrameDepth(void* aAddr);
-  void AddJITFrameDepth(void* aAddr, unsigned depth);
   void SpliceFrameTableElements(SpliceableJSONWriter& aWriter);
   void SpliceStackTableElements(SpliceableJSONWriter& aWriter);
 
 private:
+  // Make sure that there exists a frame index for aFrame, and if there isn't
+  // one already, create one and call StreamJITFrame for the frame.
+  void MaybeAddJITFrameIndex(const FrameKey& aFrame,
+                             const JS::ProfiledFrameHandle& aJITFrame);
+
   void StreamNonJITFrame(const FrameKey& aFrame);
   void StreamJITFrame(const JS::ProfiledFrameHandle& aJITFrame);
   void StreamStack(const StackKey& aStack);
 
 public:
   UniqueJSONStrings mUniqueStrings;
 
 private:
   JSContext* mContext;
 
   // To avoid incurring JitcodeGlobalTable lookup costs for every JIT frame,
-  // we cache the depth of frames keyed by JIT code address. If an address a
-  // maps to a depth d, then frames keyed by a for depths 0 to d are
-  // guaranteed to be in mFrameToIndexMap.
-  std::map<void*, uint32_t> mJITFrameDepthMap;
+  // we cache the frame keys of frames keyed by JIT code address. All FrameKeys
+  // in mAddressToJITFrameKeysMap are guaranteed to be in mFrameToIndexMap.
+  nsClassHashtable<nsPtrHashKey<void>, nsTArray<FrameKey>> mAddressToJITFrameKeysMap;
 
-  uint32_t mFrameCount;
   SpliceableChunkedJSONWriter mFrameTableWriter;
   nsDataHashtable<nsGenericHashKey<FrameKey>, uint32_t> mFrameToIndexMap;
 
   SpliceableChunkedJSONWriter mStackTableWriter;
-
   nsDataHashtable<nsGenericHashKey<StackKey>, uint32_t> mStackToIndexMap;
 };
 
 //
 // Thread profile JSON Format
 // --------------------------
 //
 // The profile contains much duplicate information. The output JSON of the