From f008f31bf31c049367afb1edaf16ff0b01555d0e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 15 Apr 2014 22:58:44 -0400 Subject: [PATCH] Bug 995295. Make console.trace() faster when the console is closed by avoiding reification of the stack until someone actually asks for it. r=baku --- dom/base/Console.cpp | 211 ++++++++++++++++++++++++++++++++------ dom/webidl/Console.webidl | 7 +- 2 files changed, 185 insertions(+), 33 deletions(-) diff --git a/dom/base/Console.cpp b/dom/base/Console.cpp index a65df2ed92c..1ba4245eda8 100644 --- a/dom/base/Console.cpp +++ b/dom/base/Console.cpp @@ -7,6 +7,8 @@ #include "mozilla/dom/ConsoleBinding.h" #include "mozilla/dom/Exceptions.h" +#include "mozilla/dom/ToJSValue.h" +#include "mozilla/Maybe.h" #include "nsCycleCollectionParticipant.h" #include "nsDocument.h" #include "nsDOMNavigationTiming.h" @@ -16,6 +18,7 @@ #include "WorkerPrivate.h" #include "WorkerRunnable.h" #include "xpcprivate.h" +#include "nsContentUtils.h" #include "nsIConsoleAPIStorage.h" #include "nsIDOMWindowUtils.h" @@ -171,7 +174,16 @@ public: nsString mMethodString; nsTArray> mArguments; - Sequence mStack; + + // Stack management is complicated, because we want to do it as + // lazily as possible. Therefore, we have the following behavior: + // 1) mTopStackFrame is initialized whenever we have any JS on the stack + // 2) mReifiedStack is initialized if we're created in a worker. + // 3) mStack is set (possibly to null if there is no JS on the stack) if + // we're created on main thread. + Maybe mTopStackFrame; + Maybe> mReifiedStack; + nsCOMPtr mStack; }; // This class is used to clear any exception at the end of this method. @@ -734,6 +746,58 @@ Console::__noSuchMethod__() // Nothing to do. } +static +nsresult +StackFrameToStackEntry(nsIStackFrame* aStackFrame, + ConsoleStackEntry& aStackEntry, + uint32_t aLanguage) +{ + MOZ_ASSERT(aStackFrame); + + nsresult rv = aStackFrame->GetFilename(aStackEntry.mFilename); + NS_ENSURE_SUCCESS(rv, rv); + + int32_t lineNumber; + rv = aStackFrame->GetLineNumber(&lineNumber); + NS_ENSURE_SUCCESS(rv, rv); + + aStackEntry.mLineNumber = lineNumber; + + rv = aStackFrame->GetName(aStackEntry.mFunctionName); + NS_ENSURE_SUCCESS(rv, rv); + + aStackEntry.mLanguage = aLanguage; + return NS_OK; +} + +static +nsresult +ReifyStack(nsIStackFrame* aStack, nsTArray& aRefiedStack) +{ + nsCOMPtr stack(aStack); + + while (stack) { + uint32_t language; + nsresult rv = stack->GetLanguage(&language); + NS_ENSURE_SUCCESS(rv, rv); + + if (language == nsIProgrammingLanguage::JAVASCRIPT || + language == nsIProgrammingLanguage::JAVASCRIPT2) { + ConsoleStackEntry& data = *aRefiedStack.AppendElement(); + nsresult rv = StackFrameToStackEntry(stack, data, language); + NS_ENSURE_SUCCESS(rv, rv); + } + + nsCOMPtr caller; + rv = stack->GetCaller(getter_AddRefs(caller)); + NS_ENSURE_SUCCESS(rv, rv); + + stack.swap(caller); + } + + return NS_OK; +} + // Queue a call to a console method. See the CALL_DELAY constant. void Console::Method(JSContext* aCx, MethodName aMethodName, @@ -798,8 +862,7 @@ Console::Method(JSContext* aCx, MethodName aMethodName, return; } - // nsIStackFrame is not thread-safe so we take what we need and we store in - // an array of ConsoleStackEntry objects. + // Walk up to the first JS stack frame and save it if we find it. do { uint32_t language; nsresult rv = stack->GetLanguage(&language); @@ -810,30 +873,16 @@ Console::Method(JSContext* aCx, MethodName aMethodName, if (language == nsIProgrammingLanguage::JAVASCRIPT || language == nsIProgrammingLanguage::JAVASCRIPT2) { - ConsoleStackEntry& data = *callData->mStack.AppendElement(); - - rv = stack->GetFilename(data.mFilename); + callData->mTopStackFrame.construct(); + nsresult rv = StackFrameToStackEntry(stack, + callData->mTopStackFrame.ref(), + language); if (NS_FAILED(rv)) { Throw(aCx, rv); return; } - int32_t lineNumber; - rv = stack->GetLineNumber(&lineNumber); - if (NS_FAILED(rv)) { - Throw(aCx, rv); - return; - } - - data.mLineNumber = lineNumber; - - rv = stack->GetName(data.mFunctionName); - if (NS_FAILED(rv)) { - Throw(aCx, rv); - return; - } - - data.mLanguage = language; + break; } nsCOMPtr caller; @@ -846,6 +895,19 @@ Console::Method(JSContext* aCx, MethodName aMethodName, stack.swap(caller); } while (stack); + if (NS_IsMainThread()) { + callData->mStack = stack; + } else { + // nsIStackFrame is not threadsafe, so we need to snapshot it now, + // before we post our runnable to the main thread. + callData->mReifiedStack.construct(); + nsresult rv = ReifyStack(stack, callData->mReifiedStack.ref()); + if (NS_FAILED(rv)) { + Throw(aCx, rv); + return; + } + } + // Monotonic timer for 'time' and 'timeEnd' if ((aMethodName == MethodTime || aMethodName == MethodTimeEnd) && mWindow) { nsGlobalWindow *win = static_cast(mWindow.get()); @@ -919,14 +981,60 @@ Console::Notify(nsITimer *timer) return NS_OK; } +// We store information to lazily compute the stack in the reserved slots of +// LazyStackGetter. The first slot always stores a JS object: it's either the +// JS wrapper of the nsIStackFrame or the actual reified stack representation. +// The second slot is a PrivateValue() holding an nsIStackFrame* when we haven't +// reified the stack yet, or an UndefinedValue() otherwise. +enum { + SLOT_STACKOBJ, + SLOT_RAW_STACK +}; + +bool +LazyStackGetter(JSContext* aCx, unsigned aArgc, JS::Value* aVp) +{ + JS::CallArgs args = CallArgsFromVp(aArgc, aVp); + JS::Rooted callee(aCx, &args.callee()); + + JS::Value v = js::GetFunctionNativeReserved(&args.callee(), SLOT_RAW_STACK); + if (v.isUndefined()) { + // Already reified. + args.rval().set(js::GetFunctionNativeReserved(callee, SLOT_STACKOBJ)); + return true; + } + + nsIStackFrame* stack = reinterpret_cast(v.toPrivate()); + nsTArray reifiedStack; + nsresult rv = ReifyStack(stack, reifiedStack); + if (NS_FAILED(rv)) { + Throw(aCx, rv); + return false; + } + + JS::Rooted stackVal(aCx); + if (!ToJSValue(aCx, reifiedStack, &stackVal)) { + return false; + } + + MOZ_ASSERT(stackVal.isObject()); + + js::SetFunctionNativeReserved(callee, SLOT_STACKOBJ, stackVal); + js::SetFunctionNativeReserved(callee, SLOT_RAW_STACK, JS::UndefinedValue()); + + args.rval().set(stackVal); + return true; +} + void Console::ProcessCallData(ConsoleCallData* aData) { MOZ_ASSERT(aData); + MOZ_ASSERT(NS_IsMainThread()); ConsoleStackEntry frame; - if (!aData->mStack.IsEmpty()) { - frame = aData->mStack[0]; + if (!aData->mTopStackFrame.empty()) { + frame = aData->mTopStackFrame.ref(); } AutoSafeJSContext cx; @@ -972,14 +1080,9 @@ Console::ProcessCallData(ConsoleCallData* aData) ArgumentsToValueList(aData->mArguments, event.mArguments.Value()); } - if (ShouldIncludeStackrace(aData->mMethodName)) { - event.mStacktrace.Construct(); - event.mStacktrace.Value().SwapElements(aData->mStack); - } - - else if (aData->mMethodName == MethodGroup || - aData->mMethodName == MethodGroupCollapsed || - aData->mMethodName == MethodGroupEnd) { + if (aData->mMethodName == MethodGroup || + aData->mMethodName == MethodGroupCollapsed || + aData->mMethodName == MethodGroupEnd) { ComposeGroupName(cx, aData->mArguments, event.mGroupName); } @@ -1009,6 +1112,50 @@ Console::ProcessCallData(ConsoleCallData* aData) return; } + if (ShouldIncludeStackrace(aData->mMethodName)) { + // Now define the "stacktrace" property on eventObj. There are two cases + // here. Either we came from a worker and have a reified stack, or we want + // to define a getter that will lazily reify the stack. + if (!aData->mReifiedStack.empty()) { + JS::Rooted stacktrace(cx); + if (!ToJSValue(cx, aData->mReifiedStack.ref(), &stacktrace) || + !JS_DefineProperty(cx, eventObj, "stacktrace", stacktrace, + nullptr, nullptr, JSPROP_ENUMERATE)) { + return; + } + } else { + JSFunction* fun = js::NewFunctionWithReserved(cx, LazyStackGetter, 0, 0, + eventObj, "stacktrace"); + if (!fun) { + return; + } + + JS::Rooted funObj(cx, JS_GetFunctionObject(fun)); + + // We want to store our stack in the function and have it stay alive. But + // we also need sane access to the C++ nsIStackFrame. So store both a JS + // wrapper and the raw pointer: the former will keep the latter alive. + JS::Rooted stackVal(cx); + nsresult rv = nsContentUtils::WrapNative(cx, aData->mStack, + &stackVal); + if (NS_FAILED(rv)) { + return; + } + + js::SetFunctionNativeReserved(funObj, SLOT_STACKOBJ, stackVal); + js::SetFunctionNativeReserved(funObj, SLOT_RAW_STACK, + JS::PrivateValue(aData->mStack.get())); + + if (!JS_DefineProperty(cx, eventObj, "stacktrace", JS::UndefinedValue(), + JS_DATA_TO_FUNC_PTR(JSPropertyOp, funObj.get()), + nullptr, + JSPROP_ENUMERATE | JSPROP_SHARED | JSPROP_GETTER | + JSPROP_SETTER)) { + return; + } + } + } + if (!mStorage) { mStorage = do_GetService("@mozilla.org/consoleAPI-storage;1"); } diff --git a/dom/webidl/Console.webidl b/dom/webidl/Console.webidl index c069971e44c..bdaac9c8cf8 100644 --- a/dom/webidl/Console.webidl +++ b/dom/webidl/Console.webidl @@ -47,7 +47,12 @@ dictionary ConsoleEvent { sequence styles; boolean private = false; - sequence stacktrace; + // stacktrace is handled via a getter in some cases so we can construct it + // lazily. Note that we're not making this whole thing an interface because + // consumers expect to see own properties on it, which would mean making the + // props unforgeable, which means lots of JSFunction allocations. Maybe we + // should fix those consumers, of course.... + // sequence stacktrace; DOMString groupName = ""; any timer = null; any counter = null;