Bug 1434965 - Remove OnStackFrameKey and solve frame canonicalization differently. r?njn
MozReview-Commit-ID: DITzwF94U64
--- 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