Bug 162503

Summary: [Binding] setDOMException should be inlined and fall to the slow path if exception occurs
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: BindingsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cgarcia, commit-queue, fpizlo, ggaren, saam, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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>