Bug 1434965 - Take 'streaming generation' into account when de-duplicating JIT frames. r?njn
MozReview-Commit-ID: KlhSPY4ZrOR
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -283,30 +283,39 @@ UniqueStacks::BeginStack(const FrameKey&
}
UniqueStacks::StackKey
UniqueStacks::AppendFrame(const StackKey& aStack, const FrameKey& aFrame)
{
return StackKey(aStack, GetOrAddStackIndex(aStack), GetOrAddFrameIndex(aFrame));
}
+uint32_t
+UniqueStacks::JITAddress::Hash() const
+{
+ uint32_t hash = 0;
+ hash = AddToHash(hash, mAddress);
+ hash = AddToHash(hash, mStreamingGen);
+ return hash;
+}
+
uint32_t UniqueStacks::FrameKey::Hash() const
{
uint32_t hash = 0;
if (!mLocation.IsEmpty()) {
hash = mozilla::HashString(mLocation.get());
}
if (mLine.isSome()) {
hash = mozilla::AddToHash(hash, *mLine);
}
if (mCategory.isSome()) {
hash = mozilla::AddToHash(hash, *mCategory);
}
if (mJITAddress.isSome()) {
- hash = mozilla::AddToHash(hash, *mJITAddress);
+ hash = mozilla::AddToHash(hash, mJITAddress->Hash());
if (mJITDepth.isSome()) {
hash = mozilla::AddToHash(hash, *mJITDepth);
}
}
return hash;
}
UniqueStacks::UniqueStacks()
@@ -326,27 +335,29 @@ uint32_t UniqueStacks::GetOrAddStackInde
index = mStackToIndexMap.Count();
mStackToIndexMap.Put(aStack, index);
StreamStack(aStack);
return index;
}
MOZ_MUST_USE nsTArray<UniqueStacks::FrameKey>
UniqueStacks::GetOrAddJITFrameKeysForAddress(JSContext* aContext,
- void* aJITAddress)
+ const JITAddress& aJITAddress)
{
nsTArray<FrameKey>& frameKeys =
*mAddressToJITFrameKeysMap.LookupOrAdd(aJITAddress);
if (frameKeys.IsEmpty()) {
- for (JS::ProfiledFrameHandle handle : JS::GetProfiledFrames(aContext,
- aJITAddress)) {
+ for (JS::ProfiledFrameHandle handle :
+ JS::GetProfiledFrames(aContext, aJITAddress.mAddress)) {
// 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());
+ FrameKey frameKey(
+ JITAddress{ handle.canonicalAddress(), aJITAddress.mStreamingGen },
+ frameKeys.Length());
MaybeAddJITFrameIndex(aContext, frameKey, handle);
frameKeys.AppendElement(frameKey);
}
MOZ_ASSERT(frameKeys.Length() > 0);
}
// Return a copy of the array.
return nsTArray<FrameKey>(frameKeys);
@@ -874,18 +885,19 @@ ProfileBuffer::StreamSamplesToJSON(Splic
} else if (e.Get().IsJitReturnAddr()) {
numFrames++;
// We can only process JitReturnAddr entries if we have a JSContext.
MOZ_RELEASE_ASSERT(aContext);
// A JIT frame may expand to multiple frames due to inlining.
void* pc = e.Get().u.mPtr;
+ UniqueStacks::JITAddress address = { pc, aUniqueStacks.CurrentGen() };
nsTArray<UniqueStacks::FrameKey> frameKeys =
- aUniqueStacks.GetOrAddJITFrameKeysForAddress(aContext, pc);
+ aUniqueStacks.GetOrAddJITFrameKeysForAddress(aContext, address);
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
@@ -140,23 +140,47 @@ public:
private:
SpliceableChunkedJSONWriter mStringTableWriter;
nsDataHashtable<nsCStringHashKey, uint32_t> mStringToIndexMap;
};
class UniqueStacks
{
public:
+ // We de-duplicate information about JIT frames based on the return address
+ // of the frame. However, if the same UniqueStacks object is used to stream
+ // profiler buffer contents more than once, then in the time between the two
+ // stream attempts ("streaming generations"), JIT code can have been freed
+ // and reallocated in the same areas of memory. Consequently, during the next
+ // streaming generation, we may see JIT return addresses that we've already
+ // seen before, but which now represent completely different JS functions.
+ // So we need to make sure that any de-duplication takes the streaming
+ // generation into account and does not only compare the address itself.
+ // The JITAddress struct packages the two together and can be used as a hash
+ // key.
+ struct JITAddress
+ {
+ void* mAddress;
+ uint32_t mStreamingGen;
+
+ uint32_t Hash() const;
+ bool operator==(const JITAddress& aRhs) const
+ {
+ return mAddress == aRhs.mAddress && mStreamingGen == aRhs.mStreamingGen;
+ }
+ bool operator!=(const JITAddress& aRhs) const { return !(*this == aRhs); }
+ };
+
struct FrameKey {
// This cannot be a std::string, as it is not memmove compatible, which
// is used by nsHashTable
nsCString mLocation;
mozilla::Maybe<unsigned> mLine;
mozilla::Maybe<unsigned> mCategory;
- mozilla::Maybe<void*> mJITAddress;
+ mozilla::Maybe<JITAddress> mJITAddress;
mozilla::Maybe<uint32_t> mJITDepth;
explicit FrameKey(const char* aLocation)
: mLocation(aLocation)
{
mHash = Hash();
}
@@ -165,17 +189,17 @@ public:
, mLine(aToCopy.mLine)
, mCategory(aToCopy.mCategory)
, mJITAddress(aToCopy.mJITAddress)
, mJITDepth(aToCopy.mJITDepth)
{
mHash = Hash();
}
- FrameKey(void* aJITAddress, uint32_t aJITDepth)
+ FrameKey(const JITAddress& aJITAddress, uint32_t aJITDepth)
: mJITAddress(mozilla::Some(aJITAddress))
, mJITDepth(mozilla::Some(aJITDepth))
{
mHash = Hash();
}
uint32_t Hash() const;
bool operator==(const FrameKey& aOther) const;
@@ -215,25 +239,32 @@ public:
}
private:
uint32_t mHash;
};
explicit UniqueStacks();
+ // Needs to be called when using a UniqueStacks object again after having
+ // streamed entries that are no longer in the buffer (and which could have
+ // been GC'ed and their memory reused).
+ void AdvanceStreamingGeneration() { mStreamingGeneration++; }
+ uint32_t CurrentGen() { return mStreamingGeneration; }
+
// Return a StackKey for aFrame as the stack's root frame (no prefix).
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 FrameKey& aFrame);
MOZ_MUST_USE nsTArray<FrameKey>
- GetOrAddJITFrameKeysForAddress(JSContext* aContext, void* aJITAddress);
+ GetOrAddJITFrameKeysForAddress(JSContext* aContext,
+ const JITAddress& aJITAddress);
MOZ_MUST_USE uint32_t GetOrAddFrameIndex(const FrameKey& aFrame);
MOZ_MUST_USE uint32_t GetOrAddStackIndex(const StackKey& aStack);
void SpliceFrameTableElements(SpliceableJSONWriter& aWriter);
void SpliceStackTableElements(SpliceableJSONWriter& aWriter);
private:
@@ -250,23 +281,27 @@ private:
public:
UniqueJSONStrings mUniqueStrings;
private:
// To avoid incurring JitcodeGlobalTable lookup costs for every JIT frame,
// 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;
+ nsClassHashtable<nsGenericHashKey<JITAddress>, nsTArray<FrameKey>> mAddressToJITFrameKeysMap;
SpliceableChunkedJSONWriter mFrameTableWriter;
nsDataHashtable<nsGenericHashKey<FrameKey>, uint32_t> mFrameToIndexMap;
SpliceableChunkedJSONWriter mStackTableWriter;
nsDataHashtable<nsGenericHashKey<StackKey>, uint32_t> mStackToIndexMap;
+
+ // Used to avoid collisions between JITAddresses that refer to different
+ // frames.
+ uint32_t mStreamingGeneration;
};
//
// Thread profile JSON Format
// --------------------------
//
// The profile contains much duplicate information. The output JSON of the
// profile attempts to deduplicate strings, frames, and stack prefixes, to cut
--- a/tools/profiler/core/ThreadInfo.cpp
+++ b/tools/profiler/core/ThreadInfo.cpp
@@ -78,16 +78,18 @@ ThreadInfo::StreamJSON(const ProfileBuff
const TimeStamp& aProcessStartTime, double aSinceTime)
{
UniquePtr<PartialThreadProfile> partialProfile = Move(mPartialProfile);
UniquePtr<UniqueStacks> uniqueStacks = partialProfile
? Move(partialProfile->mUniqueStacks)
: MakeUnique<UniqueStacks>();
+ uniqueStacks->AdvanceStreamingGeneration();
+
UniquePtr<char[]> partialSamplesJSON;
UniquePtr<char[]> partialMarkersJSON;
if (partialProfile) {
partialSamplesJSON = Move(partialProfile->mSamplesJSON);
partialMarkersJSON = Move(partialProfile->mMarkersJSON);
}
aWriter.Start();
@@ -244,16 +246,18 @@ ThreadInfo::FlushSamplesAndMarkers(const
// an existing array.
//
// Note that the UniqueStacks instance is persisted so that the frame-index
// mapping is stable across JS shutdown.
UniquePtr<UniqueStacks> uniqueStacks = mPartialProfile
? Move(mPartialProfile->mUniqueStacks)
: MakeUnique<UniqueStacks>();
+ uniqueStacks->AdvanceStreamingGeneration();
+
UniquePtr<char[]> samplesJSON;
UniquePtr<char[]> markersJSON;
{
SpliceableChunkedJSONWriter b;
b.StartBareList();
bool haveSamples = false;
{