Bug 162503 - [Binding] setDOMException should be inlined and fall to the slow path if exception occurs
Summary: [Binding] setDOMException should be inlined and fall to the slow path if exce...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-23 12:35 PDT by Yusuke Suzuki
Modified: 2016-09-25 00:03 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.88 KB, patch)
2016-09-23 17:37 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (13.06 KB, patch)
2016-09-23 17:41 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (13.08 KB, patch)
2016-09-23 18:06 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2016-09-23 12:35:23 PDT
setDOMException is not inlined in binding function.
Since exception rarely occurs, we should inline setDOMException like this.

inline setDOMException()
{
    if (LIKELY(error does not occur))
       return;
    setDOMExceptionSlow();  // never inline.
}
Comment 1 Yusuke Suzuki 2016-09-23 16:56:48 PDT
3794 EncodedJSValue JSC_HOST_CALL jsWebGLRenderingContextBasePrototypeFunctionUniform1f(ExecState* state)
3795 {
3796     VM& vm = state->vm();
3797     auto throwScope = DECLARE_THROW_SCOPE(vm);
3798     UNUSED_PARAM(throwScope);
3799     JSValue thisValue = state->thisValue();
3800     auto castedThis = jsDynamicCast<JSWebGLRenderingContextBase*>(thisValue);
3801     if (UNLIKELY(!castedThis))
3802         return throwThisTypeError(*state, throwScope, "WebGLRenderingContextBase", "uniform1f");
3803     ASSERT_GC_OBJECT_INHERITS(castedThis, JSWebGLRenderingContextBase::info());
3804     auto& impl = castedThis->wrapped();
3805     if (UNLIKELY(state->argumentCount() < 2))
3806         return throwVMError(state, throwScope, createNotEnoughArgumentsError(state));
3807     ExceptionCode ec = 0;
3808     WebGLUniformLocation* location = nullptr;
3809     if (!state->argument(0).isUndefinedOrNull()) {
3810         location = JSWebGLUniformLocation::toWrapped(state->uncheckedArgument(0));
3811         if (UNLIKELY(!location))
3812             return throwArgumentTypeError(*state, throwScope, 0, "location", "WebGLRenderingContextBase", "uniform1f", "WebGLUniformLocation");
3813     }
3814     auto x = convert<float>(*state, state->argument(1), ShouldAllowNonFinite::Yes);
3815     if (UNLIKELY(throwScope.exception()))
3816         return JSValue::encode(jsUndefined());
3817     impl.uniform1f(WTFMove(location), WTFMove(x), ec);
3818     setDOMException(state, ec);
3819     return JSValue::encode(jsUndefined());
3820 }

L3818, in the critical path, setDOMException is called. However, it is rare that an exception is raised. And This function is not inlined currently.
Comment 2 Yusuke Suzuki 2016-09-23 17:37:07 PDT
Created attachment 289724 [details]
Patch
Comment 3 Yusuke Suzuki 2016-09-23 17:41:57 PDT
Created attachment 289725 [details]
Patch
Comment 4 Yusuke Suzuki 2016-09-23 18:06:36 PDT
Created attachment 289729 [details]
Patch
Comment 5 Saam Barati 2016-09-24 15:34:09 PDT
Comment on attachment 289729 [details]
Patch

r=me
Comment 6 Yusuke Suzuki 2016-09-24 17:24:09 PDT
Comment on attachment 289729 [details]
Patch

Thank you!
Comment 7 WebKit Commit Bot 2016-09-24 17:45:56 PDT
Comment on attachment 289729 [details]
Patch

Clearing flags on attachment: 289729

Committed r206354: <http://trac.webkit.org/changeset/206354>
Comment 8 WebKit Commit Bot 2016-09-24 17:46:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Yusuke Suzuki 2016-09-25 00:03:58 PDT
Committed r206360: <http://trac.webkit.org/changeset/206360>