Bug 127910 – Avoid eagerly creating the JSActivation when the debugger is attached

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

This results in about a 2x speed up on Octane.  Patch coming.

[Mark Lam]

Created attachment 222693 [details]
work in progress: still need to analyze a few test failures.

[Mark Lam]

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 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 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]

Thanks for the reviews.  Suggested feedback has been applied.  Landed in r163223: <http://trac.webkit.org/r163223>.