RESOLVED FIXED 127910
Avoid eagerly creating the JSActivation when the debugger is attached
https://bugs.webkit.org/show_bug.cgi?id=127910
Summary Avoid eagerly creating the JSActivation when the debugger is attached
Mark Lam
Reported 2014-01-30 08:40:42 PST
This results in about a 2x speed up on Octane. Patch coming.
Attachments
work in progress: still need to analyze a few test failures. (36.19 KB, patch)
2014-01-30 11:17 PST, Mark Lam
no flags
the patch. (40.22 KB, patch)
2014-01-31 16:39 PST, Mark Lam
oliver: review+
Mark Lam
Comment 1 2014-01-30 11:17:29 PST
Created attachment 222693 [details] work in progress: still need to analyze a few test failures.
Mark Lam
Comment 2 2014-01-31 16:39:15 PST
Created attachment 222864 [details] the patch. This patch has passed all the layout tests with and without debugger bytecode generated. I've also done a smoke test of using the WebInspector to step through some code. Octane scores for this patch: baseline w/o WebInspector: 11621 patched w/o WebInspector: 11801 baseline w/ WebInspector: 3295 patched w/ WebInspector: 7070 2.1x improvement
Oliver Hunt
Comment 3 2014-01-31 16:42:29 PST
Comment on attachment 222864 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=222864&action=review > Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp:119 > + JSActivation* activation; > + activation = JSActivation::create(*codeBlock->vm(), m_callFrame, codeBlock); one line > Source/JavaScriptCore/interpreter/CallFrameInlines.h:145 > + ASSERT(codeBlock->needsActivation()); i'm paranoid make this a RELEASE_ASSERT as it doesn't look like it should be hot and the activation register call will mean you're going to have decent memory locality
Geoffrey Garen
Comment 4 2014-01-31 16:49:26 PST
Comment on attachment 222864 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=222864&action=review > Source/JavaScriptCore/runtime/JSScope.h:66 > +inline const char* resolveModeName(ResolveMode mode) > +{ > + static const char* const names[] = { > + "ThrowIfNotFound", > + "DoNotThrowIfNotFound" > + }; > + return names[mode]; > +} > + > +inline const char* resolveTypeName(ResolveType type) I think that inlining these will cause copies of your tables in places. Let's put these into the .cpp file.
Mark Lam
Comment 5 2014-01-31 17:21:02 PST
Thanks for the reviews. Suggested feedback has been applied. Landed in r163223: <http://trac.webkit.org/r163223>.
Note You need to log in before you can comment on or make changes to this bug.