| Summary: | Avoid eagerly creating the JSActivation when the debugger is attached | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
| Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | fpizlo, ggaren, joepeck, mhahnenberg, msaboff, oliver, timothy | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 128024 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Mark Lam
2014-01-30 08:40:42 PST
Created attachment 222693 [details]
work in progress: still need to analyze a few test failures.
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
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 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. Thanks for the reviews. Suggested feedback has been applied. Landed in r163223: <http://trac.webkit.org/r163223>. |