Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201975 => 201976)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-11 19:58:07 UTC (rev 201976)
@@ -1,5 +1,48 @@
2016-06-11 Mark Lam <[email protected]>
+ Minimize the amount of memcpy done for allocating Error stacks.
+ https://bugs.webkit.org/show_bug.cgi?id=158664
+
+ Reviewed by Darin Adler.
+
+ Currently, Vector<StackFrame> are being copied around multiple times in the
+ process of creating Error stacks.
+
+ This patch avoids this unnecessary copying by:
+ 1. Sizing the StackFrame vector correctly to begin with, and skipping
+ undesirable top frames before filling in the vector.
+ 2. Using perfect forwarding or passing by reference to pass the vector data around
+ instead of copying the vectors.
+ 3. Changing the Exception object to take a Vector<StackFrame> instead of a
+ RefCountedArray<StackFrame>.
+
+ This patch has passed the JSC and layout tests. Benchmarks show that perf is
+ neutral.
+
+ * API/tests/testapi.mm:
+ (testObjectiveCAPI):
+ * inspector/ScriptCallStackFactory.cpp:
+ (Inspector::createScriptCallStackFromException):
+ * interpreter/Interpreter.cpp:
+ (JSC::GetStackTraceFunctor::GetStackTraceFunctor):
+ (JSC::GetStackTraceFunctor::operator()):
+ (JSC::Interpreter::getStackTrace):
+ (JSC::Interpreter::stackTraceAsString):
+ (JSC::findExceptionHandler):
+ * interpreter/Interpreter.h:
+ * runtime/Error.cpp:
+ (JSC::addErrorInfoAndGetBytecodeOffset):
+ * runtime/Exception.cpp:
+ (JSC::Exception::finishCreation):
+ * runtime/Exception.h:
+ (JSC::Exception::valueOffset):
+ (JSC::Exception::value):
+ (JSC::Exception::stack):
+ (JSC::Exception::didNotifyInspectorOfThrow):
+ (JSC::Exception::setDidNotifyInspectorOfThrow):
+
+2016-06-11 Mark Lam <[email protected]>
+
Tests that overflows the stack should not be run with the sampling profiler.
https://bugs.webkit.org/show_bug.cgi?id=158663
Modified: trunk/Source/_javascript_Core/inspector/ScriptCallStackFactory.cpp (201975 => 201976)
--- trunk/Source/_javascript_Core/inspector/ScriptCallStackFactory.cpp 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/inspector/ScriptCallStackFactory.cpp 2016-06-11 19:58:07 UTC (rev 201976)
@@ -137,7 +137,7 @@
Ref<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState* exec, JSC::Exception* exception, size_t maxStackSize)
{
Vector<ScriptCallFrame> frames;
- RefCountedArray<StackFrame> stackTrace = exception->stack();
+ auto& stackTrace = exception->stack();
VM& vm = exec->vm();
for (size_t i = 0; i < stackTrace.size() && i < maxStackSize; i++) {
unsigned line;
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (201975 => 201976)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-06-11 19:58:07 UTC (rev 201976)
@@ -498,29 +498,35 @@
class GetStackTraceFunctor {
public:
- GetStackTraceFunctor(VM& vm, Vector<StackFrame>& results, size_t remainingCapacity)
+ GetStackTraceFunctor(VM& vm, Vector<StackFrame>& results, size_t framesToSkip, size_t capacity)
: m_vm(vm)
, m_results(results)
- , m_remainingCapacityForFrameCapture(remainingCapacity)
+ , m_framesToSkip(framesToSkip)
+ , m_remainingCapacityForFrameCapture(capacity)
{
+ m_results.reserveInitialCapacity(capacity);
}
StackVisitor::Status operator()(StackVisitor& visitor) const
{
- VM& vm = m_vm;
+ if (m_framesToSkip > 0) {
+ m_framesToSkip--;
+ return StackVisitor::Continue;
+ }
+
if (m_remainingCapacityForFrameCapture) {
if (visitor->isJSFrame()
&& !isWebAssemblyExecutable(visitor->codeBlock()->ownerExecutable())
&& !visitor->codeBlock()->unlinkedCodeBlock()->isBuiltinFunction()) {
StackFrame s = {
- Strong<JSObject>(vm, visitor->callee()),
- Strong<CodeBlock>(vm, visitor->codeBlock()),
+ Strong<JSObject>(m_vm, visitor->callee()),
+ Strong<CodeBlock>(m_vm, visitor->codeBlock()),
visitor->bytecodeOffset()
};
m_results.append(s);
} else {
StackFrame s = {
- Strong<JSObject>(vm, visitor->callee()),
+ Strong<JSObject>(m_vm, visitor->callee()),
Strong<CodeBlock>(),
0 // unused value because codeBlock is null.
};
@@ -536,31 +542,43 @@
private:
VM& m_vm;
Vector<StackFrame>& m_results;
+ mutable size_t m_framesToSkip;
mutable size_t m_remainingCapacityForFrameCapture;
};
-void Interpreter::getStackTrace(Vector<StackFrame>& results, size_t maxStackSize)
+void Interpreter::getStackTrace(Vector<StackFrame>& results, size_t framesToSkip, size_t maxStackSize)
{
VM& vm = m_vm;
CallFrame* callFrame = vm.topCallFrame;
if (!callFrame)
return;
- GetStackTraceFunctor functor(vm, results, maxStackSize);
+ size_t framesCount = 0;
+ callFrame->iterate([&] (StackVisitor&) -> StackVisitor::Status {
+ framesCount++;
+ return StackVisitor::Continue;
+ });
+ if (framesCount <= framesToSkip)
+ return;
+
+ framesCount -= framesToSkip;
+ framesCount = std::min(maxStackSize, framesCount);
+
+ GetStackTraceFunctor functor(vm, results, framesToSkip, framesCount);
callFrame->iterate(functor);
+ ASSERT(results.size() == results.capacity());
}
-JSString* Interpreter::stackTraceAsString(ExecState* exec, Vector<StackFrame> stackTrace)
+JSString* Interpreter::stackTraceAsString(VM& vm, const Vector<StackFrame>& stackTrace)
{
// FIXME: JSStringJoiner could be more efficient than StringBuilder here.
StringBuilder builder;
- VM& vm = exec->vm();
for (unsigned i = 0; i < stackTrace.size(); i++) {
builder.append(String(stackTrace[i].toString(vm)));
if (i != stackTrace.size() - 1)
builder.append('\n');
}
- return jsString(&exec->vm(), builder.toString());
+ return jsString(&vm, builder.toString());
}
ALWAYS_INLINE static HandlerInfo* findExceptionHandler(StackVisitor& visitor, CodeBlock* codeBlock, CodeBlock::RequiredHandler requiredHandler)
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.h (201975 => 201976)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.h 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.h 2016-06-11 19:58:07 UTC (rev 201976)
@@ -217,7 +217,7 @@
NEVER_INLINE HandlerInfo* unwind(VM&, CallFrame*&, Exception*, UnwindStart);
void notifyDebuggerOfExceptionToBeThrown(CallFrame*, Exception*);
NEVER_INLINE void debug(CallFrame*, DebugHookID);
- JSString* stackTraceAsString(ExecState*, Vector<StackFrame>);
+ static JSString* stackTraceAsString(VM&, const Vector<StackFrame>&);
static EncodedJSValue JSC_HOST_CALL constructWithErrorConstructor(ExecState*);
static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*);
@@ -226,7 +226,7 @@
JS_EXPORT_PRIVATE void dumpCallFrame(CallFrame*);
- void getStackTrace(Vector<StackFrame>& results, size_t maxStackSize = std::numeric_limits<size_t>::max());
+ void getStackTrace(Vector<StackFrame>& results, size_t framesToSkip = 0, size_t maxStackSize = std::numeric_limits<size_t>::max());
private:
enum ExecutionFlag { Normal, InitializeAndReturn };
Modified: trunk/Source/_javascript_Core/runtime/Error.cpp (201975 => 201976)
--- trunk/Source/_javascript_Core/runtime/Error.cpp 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/runtime/Error.cpp 2016-06-11 19:58:07 UTC (rev 201976)
@@ -143,7 +143,8 @@
{
Vector<StackFrame> stackTrace = Vector<StackFrame>();
- vm.interpreter->getStackTrace(stackTrace);
+ size_t framesToSkip = useCurrentFrame ? 0 : 1;
+ vm.interpreter->getStackTrace(stackTrace, framesToSkip);
if (!stackTrace.isEmpty()) {
ASSERT(exec == vm.topCallFrame || exec == exec->lexicalGlobalObject()->globalExec() || exec == exec->vmEntryGlobalObject()->globalExec());
@@ -173,9 +174,7 @@
if (!frameSourceURL.isEmpty())
obj->putDirect(vm, vm.propertyNames->sourceURL, jsString(&vm, frameSourceURL), ReadOnly | DontDelete);
- if (!useCurrentFrame)
- stackTrace.remove(0);
- obj->putDirect(vm, vm.propertyNames->stack, vm.interpreter->stackTraceAsString(vm.topCallFrame, stackTrace), DontEnum);
+ obj->putDirect(vm, vm.propertyNames->stack, Interpreter::stackTraceAsString(vm, stackTrace), DontEnum);
return true;
}
Modified: trunk/Source/_javascript_Core/runtime/Exception.cpp (201975 => 201976)
--- trunk/Source/_javascript_Core/runtime/Exception.cpp 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/runtime/Exception.cpp 2016-06-11 19:58:07 UTC (rev 201976)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -77,7 +77,7 @@
Vector<StackFrame> stackTrace;
if (action == StackCaptureAction::CaptureStack)
vm.interpreter->getStackTrace(stackTrace);
- m_stack = RefCountedArray<StackFrame>(stackTrace);
+ m_stack = WTFMove(stackTrace);
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/Exception.h (201975 => 201976)
--- trunk/Source/_javascript_Core/runtime/Exception.h 2016-06-11 19:48:48 UTC (rev 201975)
+++ trunk/Source/_javascript_Core/runtime/Exception.h 2016-06-11 19:58:07 UTC (rev 201976)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,7 +27,7 @@
#define Exception_h
#include "Interpreter.h"
-#include <wtf/RefCountedArray.h>
+#include <wtf/Vector.h>
namespace JSC {
@@ -57,7 +57,7 @@
}
JSValue value() const { return m_value.get(); }
- const RefCountedArray<StackFrame>& stack() const { return m_stack; }
+ const Vector<StackFrame>& stack() const { return m_stack; }
bool didNotifyInspectorOfThrow() const { return m_didNotifyInspectorOfThrow; }
void setDidNotifyInspectorOfThrow() { m_didNotifyInspectorOfThrow = true; }
@@ -69,7 +69,7 @@
void finishCreation(VM&, JSValue thrownValue, StackCaptureAction);
WriteBarrier<Unknown> m_value;
- RefCountedArray<StackFrame> m_stack;
+ Vector<StackFrame> m_stack;
bool m_didNotifyInspectorOfThrow { false };
friend class LLIntOffsetsExtractor;
