From 07f8b23350f495131e949b5aab1ebd436c5be949 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 12 Jul 2023 00:39:11 -0700 Subject: Make Error.captureStackTrace 4x faster --- src/bun.js/bindings/ErrorStackTrace.cpp | 4 +- src/bun.js/bindings/ErrorStackTrace.h | 2 +- src/bun.js/bindings/ZigGlobalObject.cpp | 151 +++++++++++++++++++++------- src/bun.js/bindings/ZigGlobalObject.h | 2 + test/js/node/v8/capture-stack-trace.test.js | 4 +- 5 files changed, 123 insertions(+), 40 deletions(-) diff --git a/src/bun.js/bindings/ErrorStackTrace.cpp b/src/bun.js/bindings/ErrorStackTrace.cpp index 59e0e765b..87627a482 100644 --- a/src/bun.js/bindings/ErrorStackTrace.cpp +++ b/src/bun.js/bindings/ErrorStackTrace.cpp @@ -19,7 +19,7 @@ using namespace WebCore; namespace Zig { -JSCStackTrace JSCStackTrace::fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames) +JSCStackTrace JSCStackTrace::fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames, int skipCount) { WTF::Vector newFrames; @@ -29,7 +29,7 @@ JSCStackTrace JSCStackTrace::fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames); + static JSCStackTrace fromExisting(JSC::VM& vm, const WTF::Vector& existingFrames, int skipCount = 0); /* This is based on JSC::Interpreter::getStackTrace, but skips native (non js and not wasm) * frames, which is what v8 does. Note that we could have just called JSC::Interpreter::getStackTrace diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 61444668f..2cc3524a6 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -117,6 +117,7 @@ #if ENABLE(REMOTE_INSPECTOR) #include "JavaScriptCore/RemoteInspectorServer.h" #endif +#include "JavaScriptCore/GetterSetter.h" using namespace Bun; @@ -334,7 +335,7 @@ extern "C" void JSCInitialize(const char* envp[], size_t envc, void (*onCrash)(c } extern "C" void* Bun__getVM(); -extern "C" JSGlobalObject* Bun__getDefaultGlobal(); +extern "C" Zig::GlobalObject* Bun__getDefaultGlobal(); // Error.captureStackTrace may cause computeErrorInfo to be called twice // Rather than figure out the plumbing in JSC, we just skip the next call @@ -2798,30 +2799,12 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return JSC::JSValue::encode(jsUndefined()); } -JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) +static JSArray* materializeFormattedStackTrace(JSC::VM& vm, JSCStackTrace& stackTrace, Zig::GlobalObject* globalObject, JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSObject* errorObject) { - GlobalObject* globalObject = reinterpret_cast(lexicalGlobalObject); - JSC::VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); - - JSC::JSValue objectArg = callFrame->argument(0); - if (!objectArg.isObject()) { - return JSC::JSValue::encode(throwTypeError(lexicalGlobalObject, scope, "invalid_argument"_s)); - } - - JSC::JSObject* errorObject = objectArg.asCell()->getObject(); - JSC::JSValue caller = callFrame->argument(1); - - size_t stackTraceLimit = globalObject->stackTraceLimit().value(); - if (stackTraceLimit == 0) { - stackTraceLimit = DEFAULT_ERROR_STACK_TRACE_LIMIT; - } - - JSCStackTrace stackTrace = JSCStackTrace::captureCurrentJSStackTrace(globalObject, callFrame, stackTraceLimit, caller); - // Note: we cannot use tryCreateUninitializedRestricted here because we cannot allocate memory inside initializeIndex() JSC::JSArray* callSites = JSC::JSArray::create(vm, - globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), + lexicalGlobalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), stackTrace.size()); // Create the call sites (one per frame) @@ -2867,33 +2850,129 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb } } - JSC::JSValue formattedStackTrace = globalObject->formatStackTrace(vm, lexicalGlobalObject, errorObject, callSites); - RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode({})); + return callSites; +} - bool orignialSkipNextComputeErrorInfo = skipNextComputeErrorInfo; - skipNextComputeErrorInfo = true; - if (errorObject->hasProperty(lexicalGlobalObject, vm.propertyNames->stack)) { - skipNextComputeErrorInfo = true; - errorObject->deleteProperty(lexicalGlobalObject, vm.propertyNames->stack); +JSC_DEFINE_HOST_FUNCTION(jsFunctionCallStackTraceAccessor, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto& vm = globalObject->vm(); + + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSValue thisValue = callFrame->thisValue(); + + if (!thisValue) { + return JSValue::encode(jsUndefined()); } - skipNextComputeErrorInfo = orignialSkipNextComputeErrorInfo; + + auto* instance = jsDynamicCast(thisValue); + JSObject* errorObject; + JSCStackTrace stackTrace; + + if (!instance) { + JSC::Exception* exception = jsCast(thisValue); + stackTrace = JSCStackTrace::fromExisting(vm, exception->stack()); + errorObject = exception->value().getObject(); + } else { + stackTrace = JSCStackTrace::fromExisting(vm, *instance->stackTrace()); + errorObject = instance; + } + + Zig::GlobalObject* zigGlobal = jsDynamicCast(globalObject); + if (!zigGlobal) { + zigGlobal = Bun__getDefaultGlobal(); + } + + JSArray* callSites = materializeFormattedStackTrace(vm, stackTrace, zigGlobal, globalObject, errorObject); + JSValue formattedStackTrace = zigGlobal->formatStackTrace(vm, globalObject, errorObject, callSites); if (formattedStackTrace.isUndefinedOrNull()) { formattedStackTrace = JSC::jsUndefined(); } errorObject->putDirect(vm, vm.propertyNames->stack, formattedStackTrace, 0); + RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSValue {})); + + return JSC::JSValue::encode(formattedStackTrace); +} + +JSC_DEFINE_HOST_FUNCTION(jsFunctionCallStackTraceSetter, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); + + JSC::JSValue thisValue = callFrame->thisValue(); + + if (!thisValue || !thisValue.isObject()) { + return JSValue::encode(jsUndefined()); + } + + JSObject* object = thisValue.getObject(); + object->putDirect(globalObject->vm(), globalObject->vm().propertyNames->stack, callFrame->argument(0), 0); + return JSValue::encode(jsUndefined()); +} + +JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame)) +{ + GlobalObject* globalObject = reinterpret_cast(lexicalGlobalObject); + JSC::VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSC::JSValue objectArg = callFrame->argument(0); + if (!objectArg.isObject()) { + return JSC::JSValue::encode(throwTypeError(lexicalGlobalObject, scope, "invalid_argument"_s)); + } + + JSC::JSObject* errorObject = objectArg.asCell()->getObject(); + JSC::JSValue caller = callFrame->argument(1); + + { + bool prevSkipComputeNextErrorInfo = skipNextComputeErrorInfo; + skipNextComputeErrorInfo = true; + if (errorObject->hasProperty(globalObject, vm.propertyNames->stack)) { + skipNextComputeErrorInfo = true; + errorObject->deleteProperty(globalObject, vm.propertyNames->stack); + } + skipNextComputeErrorInfo = prevSkipComputeNextErrorInfo; + } + + JSValue valueToHoist = jsUndefined(); if (auto* instance = jsDynamicCast(errorObject)) { - // we make a separate copy of the StackTrace unfortunately so that we - // can later console.log it without losing the info - // - // This is not good. We should remove this in the future as it strictly makes this function - // already slower than necessary. instance->captureStackTrace(vm, globalObject, 1, false); + valueToHoist = instance; } - RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSValue {})); + if (!valueToHoist.isUndefined()) { + size_t stackTraceLimit = globalObject->stackTraceLimit().value(); + if (stackTraceLimit == 0) { + stackTraceLimit = DEFAULT_ERROR_STACK_TRACE_LIMIT; + } + Options::exceptionStackTraceLimit() = stackTraceLimit; + JSC::Exception* exception = JSC::Exception::create(vm, errorObject, JSC::Exception::StackCaptureAction::CaptureStack); + valueToHoist = exception; + } + + JSFunction* callStackTraceSetter; + JSFunction* callStackTraceAccessor; + if (auto val = globalObject->m_callStackTraceSetter.get()) { + callStackTraceSetter = jsCast(val); + } else { + callStackTraceSetter = JSFunction::create(vm, globalObject, 0, String(), jsFunctionCallStackTraceSetter, ImplementationVisibility::Public); + globalObject->m_callStackTraceSetter.set(vm, globalObject, callStackTraceSetter); + } + + if (auto val = globalObject->m_callStackTraceAccessor.get()) { + callStackTraceAccessor = jsCast(val); + } else { + callStackTraceAccessor = JSFunction::create(vm, globalObject, 0, String(), jsFunctionCallStackTraceAccessor, ImplementationVisibility::Public); + globalObject->m_callStackTraceAccessor.set(vm, globalObject, callStackTraceAccessor); + } + + JSBoundFunction* boundCallStackTraceAccessor = JSBoundFunction::create(vm, globalObject, callStackTraceAccessor, valueToHoist, ArgList(), 0, nullptr); + JSC::GetterSetter* getterSetter = JSC::GetterSetter::create(vm, globalObject, boundCallStackTraceAccessor, callStackTraceSetter); + + errorObject->putDirectAccessor(globalObject, vm.propertyNames->stack, getterSetter, JSC::PropertyAttribute::Accessor | 0); + RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSC::jsUndefined())); return JSC::JSValue::encode(JSC::jsUndefined()); } @@ -4226,6 +4305,8 @@ void GlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor) visitor.append(thisObject->m_JSTextEncoderSetterValue); visitor.append(thisObject->m_JSURLSearchParamsSetterValue); visitor.append(thisObject->m_JSDOMFormDataSetterValue); + visitor.append(thisObject->m_callStackTraceSetter); + visitor.append(thisObject->m_callStackTraceAccessor); thisObject->m_JSArrayBufferSinkClassStructure.visit(visitor); thisObject->m_JSBufferListClassStructure.visit(visitor); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 0b5c882f5..cd78c51ba 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -367,6 +367,8 @@ public: mutable WriteBarrier m_JSWebSocketSetterValue; mutable WriteBarrier m_JSDOMFormDataSetterValue; mutable WriteBarrier m_BunCommonJSModuleValue; + mutable WriteBarrier m_callStackTraceSetter; + mutable WriteBarrier m_callStackTraceAccessor; mutable WriteBarrier m_thenables[promiseFunctionsSize + 1]; diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index cb2624681..789a499eb 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -174,12 +174,12 @@ test("prepare stack trace", () => { let e = { message: "bad error!" }; let prevPrepareStackTrace = Error.prepareStackTrace; Error.prepareStackTrace = (e, s) => { - expect(e.message === "bad error!").toBe(true); + expect(e.message).toBe("bad error!"); expect(s.length).toBe(4); }; Error.stackTraceLimit = 10; Error.captureStackTrace(e); - expect(e.stack === undefined).toBe(true); + expect(e.stack).toBeUndefined(); Error.prepareStackTrace = prevPrepareStackTrace; } -- cgit v1.2.3