From 9ed090fe943aa15f1735426ecb37bbc0561c3d5a Mon Sep 17 00:00:00 2001 From: Michael Layzell Date: Mon, 28 Aug 2017 13:34:34 -0400 Subject: [PATCH] Bug 1394494 - Use a custom Frame::Kind for special BHR frame types, r=froydnj --- .../backgroundhangmonitor/HangDetails.cpp | 62 +++++++++++++++---- .../backgroundhangmonitor/HangStack.cpp | 19 ++++++ .../backgroundhangmonitor/HangStack.h | 33 ++++++++++ .../ThreadStackHelper.cpp | 32 +++++----- 4 files changed, 121 insertions(+), 25 deletions(-) diff --git a/toolkit/components/backgroundhangmonitor/HangDetails.cpp b/toolkit/components/backgroundhangmonitor/HangDetails.cpp index 1459906657a7..92827ad45027 100644 --- a/toolkit/components/backgroundhangmonitor/HangDetails.cpp +++ b/toolkit/components/backgroundhangmonitor/HangDetails.cpp @@ -70,6 +70,30 @@ nsHangDetails::GetAnnotations(JSContext* aCx, JS::MutableHandleValue aVal) return NS_OK; } +namespace { + +nsresult +StringFrame(JSContext* aCx, + JS::RootedObject& aTarget, + size_t aIndex, + const char* aString) +{ + JSString* jsString = JS_NewStringCopyZ(aCx, aString); + if (!jsString) { + return NS_ERROR_OUT_OF_MEMORY; + } + JS::RootedString string(aCx, jsString); + if (!string) { + return NS_ERROR_OUT_OF_MEMORY; + } + if (!JS_DefineElement(aCx, aTarget, aIndex, string, JSPROP_ENUMERATE)) { + return NS_ERROR_OUT_OF_MEMORY; + } + return NS_OK; +} + +} // anonymous namespace + NS_IMETHODIMP nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle aVal) { @@ -81,17 +105,8 @@ nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle aVal) const HangStack::Frame& frame = mDetails.mStack[i]; switch (frame.GetKind()) { case HangStack::Frame::Kind::STRING: { - JSString* jsString = JS_NewStringCopyZ(aCx, frame.AsString()); - if (!jsString) { - return NS_ERROR_OUT_OF_MEMORY; - } - JS::RootedString string(aCx, jsString); - if (!string) { - return NS_ERROR_OUT_OF_MEMORY; - } - if (!JS_DefineElement(aCx, ret, i, string, JSPROP_ENUMERATE)) { - return NS_ERROR_OUT_OF_MEMORY; - } + nsresult rv = StringFrame(aCx, ret, i, frame.AsString()); + NS_ENSURE_SUCCESS(rv, rv); break; } @@ -138,6 +153,31 @@ nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle aVal) } break; } + + case HangStack::Frame::Kind::CONTENT: { + nsresult rv = StringFrame(aCx, ret, i, "(content script)"); + NS_ENSURE_SUCCESS(rv, rv); + break; + } + + case HangStack::Frame::Kind::JIT: { + nsresult rv = StringFrame(aCx, ret, i, "(jit frame)"); + NS_ENSURE_SUCCESS(rv, rv); + break; + } + + case HangStack::Frame::Kind::WASM: { + nsresult rv = StringFrame(aCx, ret, i, "(wasm)"); + NS_ENSURE_SUCCESS(rv, rv); + break; + } + + case HangStack::Frame::Kind::SUPPRESSED: { + nsresult rv = StringFrame(aCx, ret, i, "(profiling suppressed)"); + NS_ENSURE_SUCCESS(rv, rv); + break; + } + default: MOZ_ASSERT_UNREACHABLE("Invalid variant"); break; diff --git a/toolkit/components/backgroundhangmonitor/HangStack.cpp b/toolkit/components/backgroundhangmonitor/HangStack.cpp index dd6a4d8f0cff..8315d8d1dc9b 100644 --- a/toolkit/components/backgroundhangmonitor/HangStack.cpp +++ b/toolkit/components/backgroundhangmonitor/HangStack.cpp @@ -227,6 +227,13 @@ ParamTraits::Write(Message* aMsg, const mozilla::HangStack& WriteParam(aMsg, frame.AsPC()); break; } + case Frame::Kind::CONTENT: + case Frame::Kind::WASM: + case Frame::Kind::JIT: + case Frame::Kind::SUPPRESSED: { + // NOTE: no associated data. + break; + } default: { MOZ_RELEASE_ASSERT(false, "Invalid kind for HangStack Frame"); break; @@ -285,6 +292,18 @@ ParamTraits::Read(const Message* aMsg, aResult->infallibleAppend(Frame(pc)); break; } + case Frame::Kind::CONTENT: + aResult->infallibleAppend(Frame::Content()); + break; + case Frame::Kind::WASM: + aResult->infallibleAppend(Frame::Wasm()); + break; + case Frame::Kind::JIT: + aResult->infallibleAppend(Frame::Jit()); + break; + case Frame::Kind::SUPPRESSED: + aResult->infallibleAppend(Frame::Suppressed()); + break; default: // We can't deserialize other kinds! return false; diff --git a/toolkit/components/backgroundhangmonitor/HangStack.h b/toolkit/components/backgroundhangmonitor/HangStack.h index aaf25f104786..96c8c7c98612 100644 --- a/toolkit/components/backgroundhangmonitor/HangStack.h +++ b/toolkit/components/backgroundhangmonitor/HangStack.h @@ -44,6 +44,10 @@ public: // * Kind::STRING(const char*) : A string representing a pseudostack or chrome JS stack frame. // * Kind::MODOFFSET(ModOffset) : A module index and offset into that module. // * Kind::PC(uintptr_t) : A raw program counter which has not been mapped to a module. + // * Kind::CONTENT: A hidden "(content script)" frame. + // * Kind::JIT : An unprocessed "(jit frame)". + // * Kind::WASM : An unprocessed "(wasm)" frame. + // * Kind::SUPPRESSED : A JS frame while profiling was suppressed. // // NOTE: A manually rolled tagged enum is used instead of mozilla::Variant // here because we cannot use mozilla::Variant's IPC serialization directly. @@ -57,6 +61,10 @@ public: STRING, MODOFFSET, PC, + CONTENT, + JIT, + WASM, + SUPPRESSED, END // Marker }; @@ -104,7 +112,32 @@ public: return mPC; } + // Public constant frames copies of each of the data-less frames. + static Frame Content() { + return Frame(Kind::CONTENT); + } + static Frame Jit() { + return Frame(Kind::JIT); + } + static Frame Wasm() { + return Frame(Kind::WASM); + } + static Frame Suppressed() { + return Frame(Kind::SUPPRESSED); + } + private: + explicit Frame(Kind aKind) + : mKind(aKind) + { + MOZ_ASSERT(aKind == Kind::CONTENT || + aKind == Kind::JIT || + aKind == Kind::WASM || + aKind == Kind::SUPPRESSED, + "Kind must only be one of CONTENT, JIT, WASM or SUPPRESSED " + "for the data-free constructor."); + } + Kind mKind; union { const char* mString; diff --git a/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp b/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp index beb11ebf2922..5a2c0f8522fc 100644 --- a/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp +++ b/toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp @@ -165,6 +165,20 @@ ThreadStackHelper::TryAppendFrame(HangStack::Frame aFrame) { MOZ_RELEASE_ASSERT(mStackToFill); + // We deduplicate identical frames of kind CONTENT, JIT, WASM, and SUPPRESSED. + switch (aFrame.GetKind()) { + case HangStack::Frame::Kind::CONTENT: + case HangStack::Frame::Kind::JIT: + case HangStack::Frame::Kind::WASM: + case HangStack::Frame::Kind::SUPPRESSED: + if (!mStackToFill->empty() && mStackToFill->back().GetKind() == aFrame.GetKind()) { + return; + } + break; + default: + break; + } + // Record that we _want_ to use another frame entry. If this exceeds // mMaxStackSize, we'll allocate more room on the next hang. mDesiredStackSize += 1; @@ -186,7 +200,7 @@ void ThreadStackHelper::CollectJitReturnAddr(void* aAddr) { MOZ_RELEASE_ASSERT(mStackToFill); - TryAppendFrame(HangStack::Frame("(jit frame)")); + TryAppendFrame(HangStack::Frame::Jit()); } void @@ -195,7 +209,7 @@ ThreadStackHelper::CollectWasmFrame(const char* aLabel) MOZ_RELEASE_ASSERT(mStackToFill); // We don't want to collect WASM frames, as they are probably for content, so // we just add a "(content wasm)" frame. - TryAppendFrame(HangStack::Frame("(wasm)")); + TryAppendFrame(HangStack::Frame::Wasm()); } namespace { @@ -247,10 +261,6 @@ GetPathAfterComponent(const char* filename, const char (&component)[LEN]) { void ThreadStackHelper::CollectPseudoEntry(const js::ProfileEntry& aEntry) { - HangStack::Frame* backFrame = mStackToFill->empty() - ? nullptr - : &mStackToFill->back(); - // For non-js frames we just include the raw label. if (!aEntry.isJs()) { const char* label = aEntry.label(); @@ -259,18 +269,12 @@ ThreadStackHelper::CollectPseudoEntry(const js::ProfileEntry& aEntry) } if (!aEntry.script()) { - TryAppendFrame(HangStack::Frame("(profiling suppressed)")); + TryAppendFrame(HangStack::Frame::Suppressed()); return; } if (!IsChromeJSScript(aEntry.script())) { - // NOTE: Deduplicate consecutive (content script) frames. - if (backFrame && - backFrame->GetKind() == HangStack::Frame::Kind::STRING && - !strcmp(backFrame->AsString(), "(content script)")) { - return; - } - TryAppendFrame(HangStack::Frame("(content script)")); + TryAppendFrame(HangStack::Frame::Content()); return; }