Bug 1785745 - Make UniqueStack::mUniqueStrings and mCodeAddressService private - r=canaltinova

This makes it easier to understand how these are actually used, and constrain
these uses as intended.

Differential Revision: https://phabricator.services.mozilla.com/D154960
This commit is contained in:
Gerald Squelart 2022-08-22 21:31:48 +00:00
parent f4a903ae13
commit 90b45a2617
5 changed files with 47 additions and 33 deletions

View file

@ -204,14 +204,17 @@ class UniqueStacks {
void SpliceFrameTableElements(SpliceableJSONWriter& aWriter);
void SpliceStackTableElements(SpliceableJSONWriter& aWriter);
UniqueJSONStrings& UniqueStrings() {
MOZ_RELEASE_ASSERT(mUniqueStrings.get());
return *mUniqueStrings;
}
private:
void StreamNonJITFrame(const FrameKey& aFrame);
void StreamStack(const StackKey& aStack);
public:
UniquePtr<UniqueJSONStrings> mUniqueStrings;
private:
SpliceableChunkedJSONWriter mFrameTableWriter;
HashMap<FrameKey, uint32_t, FrameKeyHasher> mFrameToIndexMap;

View file

@ -31,8 +31,7 @@ void ProfiledThreadData::StreamJSON(const ProfileBuffer& aBuffer,
double aSinceTime) {
UniqueStacks uniqueStacks;
MOZ_ASSERT(uniqueStacks.mUniqueStrings);
aWriter.SetUniqueStrings(*uniqueStacks.mUniqueStrings);
aWriter.SetUniqueStrings(uniqueStacks.UniqueStrings());
aWriter.Start();
{
@ -78,7 +77,7 @@ void ProfiledThreadData::StreamJSON(const ProfileBuffer& aBuffer,
aWriter.StartArrayProperty("stringTable");
{
std::move(*uniqueStacks.mUniqueStrings)
std::move(uniqueStacks.UniqueStrings())
.SpliceStringTableElements(aWriter);
}
aWriter.EndArray();

View file

@ -390,6 +390,29 @@ void UniqueStacks::SpliceStackTableElements(SpliceableJSONWriter& aWriter) {
aWriter.TakeAndSplice(mStackTableWriter.TakeChunkedWriteFunc());
}
[[nodiscard]] nsAutoCString UniqueStacks::FunctionNameOrAddress(void* aPC) {
nsAutoCString nameOrAddress;
if (!mCodeAddressService ||
!mCodeAddressService->GetFunction(aPC, nameOrAddress) ||
nameOrAddress.IsEmpty()) {
nameOrAddress.AppendASCII("0x");
// `AppendInt` only knows `uint32_t` or `uint64_t`, but because these are
// just aliases for *two* of (`unsigned`, `unsigned long`, and `unsigned
// long long`), a call with `uintptr_t` could use the third type and
// therefore would be ambiguous.
// So we want to force using exactly `uint32_t` or `uint64_t`, whichever
// matches the size of `uintptr_t`.
// (The outer cast to `uint` should then be a no-op.)
using uint = std::conditional_t<sizeof(uintptr_t) <= sizeof(uint32_t),
uint32_t, uint64_t>;
nameOrAddress.AppendInt(static_cast<uint>(reinterpret_cast<uintptr_t>(aPC)),
16);
}
return nameOrAddress;
}
void UniqueStacks::StreamStack(const StackKey& aStack) {
enum Schema : uint32_t { PREFIX = 0, FRAME = 1 };
@ -999,28 +1022,11 @@ ProfilerThreadId ProfileBuffer::DoStreamSamplesAndMarkersToJSON(
void* pc = e.Get().GetPtr();
e.Next();
nsAutoCString buf;
nsAutoCString functionNameOrAddress =
uniqueStacks.FunctionNameOrAddress(pc);
if (!uniqueStacks.mCodeAddressService ||
!uniqueStacks.mCodeAddressService->GetFunction(pc, buf) ||
buf.IsEmpty()) {
buf.AppendASCII("0x");
// `AppendInt` only knows `uint32_t` or `uint64_t`, but because
// these are just aliases for *two* of (`unsigned`, `unsigned
// long`, and `unsigned long long`), a call with `uintptr_t` could
// use the third type and therefore would be ambiguous.
// So we want to force using exactly `uint32_t` or `uint64_t`,
// whichever matches the size of `uintptr_t`.
// (The outer cast to `uint` should then be a no-op.)
using uint =
std::conditional_t<sizeof(uintptr_t) <= sizeof(uint32_t),
uint32_t, uint64_t>;
buf.AppendInt(static_cast<uint>(reinterpret_cast<uintptr_t>(pc)),
16);
}
stack = uniqueStacks.AppendFrame(stack,
UniqueStacks::FrameKey(buf.get()));
stack = uniqueStacks.AppendFrame(
stack, UniqueStacks::FrameKey(functionNameOrAddress.get()));
} else if (e.Get().IsLabel()) {
numFrames++;

View file

@ -347,16 +347,23 @@ class UniqueStacks {
void SpliceFrameTableElements(SpliceableJSONWriter& aWriter);
void SpliceStackTableElements(SpliceableJSONWriter& aWriter);
[[nodiscard]] UniqueJSONStrings& UniqueStrings() {
MOZ_RELEASE_ASSERT(mUniqueStrings.get());
return *mUniqueStrings;
}
// Find the function name at the given PC (if a ProfilerCodeAddressService was
// provided), otherwise just stringify that PC.
[[nodiscard]] nsAutoCString FunctionNameOrAddress(void* aPC);
private:
void StreamNonJITFrame(const FrameKey& aFrame);
void StreamStack(const StackKey& aStack);
public:
mozilla::UniquePtr<UniqueJSONStrings> mUniqueStrings;
ProfilerCodeAddressService* mCodeAddressService = nullptr;
private:
SpliceableChunkedJSONWriter mFrameTableWriter;
mozilla::HashMap<FrameKey, uint32_t, FrameKeyHasher> mFrameToIndexMap;

View file

@ -86,7 +86,7 @@ static void StreamTables(UniqueStacks&& aUniqueStacks, JSContext* aCx,
aWriter.StartArrayProperty("stringTable");
{
aProgressLogger.SetLocalProgress(60_pc, "Splicing string table...");
std::move(*aUniqueStacks.mUniqueStrings).SpliceStringTableElements(aWriter);
std::move(aUniqueStacks.UniqueStrings()).SpliceStringTableElements(aWriter);
aProgressLogger.SetLocalProgress(90_pc, "Spliced string table");
}
aWriter.EndArray();
@ -140,8 +140,7 @@ void ProfiledThreadData::StreamJSON(
0_pc, "Preparing unique stacks...", 10_pc,
"Prepared Unique stacks"));
MOZ_ASSERT(uniqueStacks->mUniqueStrings);
aWriter.SetUniqueStrings(*uniqueStacks->mUniqueStrings);
aWriter.SetUniqueStrings(uniqueStacks->UniqueStrings());
aWriter.Start();
{
@ -390,9 +389,9 @@ ThreadStreamingContext::ThreadStreamingContext(
aProgressLogger.CreateSubLoggerFromTo(
0_pc, "Preparing thread streaming context unique stacks...",
99_pc, "Prepared thread streaming context Unique stacks"))) {
mSamplesDataWriter.SetUniqueStrings(*mUniqueStacks->mUniqueStrings);
mSamplesDataWriter.SetUniqueStrings(mUniqueStacks->UniqueStrings());
mSamplesDataWriter.StartBareList();
mMarkersDataWriter.SetUniqueStrings(*mUniqueStacks->mUniqueStrings);
mMarkersDataWriter.SetUniqueStrings(mUniqueStacks->UniqueStrings());
mMarkersDataWriter.StartBareList();
}