[Webkit-unassigned] [Bug 171637] New: Hoist DOM binding attribute getter/setter prologue into JavaScriptCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=171637

            Bug ID: 171637
           Summary: Hoist DOM binding attribute getter/setter prologue
                    into JavaScriptCore
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Bindings
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: sam at webkit.org
                CC: cdumez at apple.com

While thinking about code size of the bindings, it occurred to me that we have a ton of mostly duplicate code in bindings in the prologue of every getter / setter.

Let's take the fgColor attribute getter on HTMLDocument as an example:

EncodedJSValue jsHTMLDocumentFgColor(ExecState* state, EncodedJSValue thisValue, PropertyName)
{
    return BindingCaller<JSHTMLDocument>::attribute<jsHTMLDocumentFgColorGetter>(state, thisValue, "fgColor");
}

static inline JSValue jsHTMLDocumentFgColorGetter(ExecState& state, JSHTMLDocument& thisObject, ThrowScope& throwScope)
{
    UNUSED_PARAM(throwScope);
    UNUSED_PARAM(state);
    auto& impl = thisObject.wrapped();
    JSValue result = toJS<IDLTreatNullAsEmptyAdaptor<IDLDOMString>>(state, impl.fgColor());
    return result;
}

The BindingsCaller function attribute(...) is the following:

template<AttributeGetterFunction getter, CastedThisErrorBehavior shouldThrow = CastedThisErrorBehavior::Throw>
static JSC::EncodedJSValue attribute(JSC::ExecState* state, JSC::EncodedJSValue thisValue, const char* attributeName)
{
    ASSERT(state);
    auto throwScope = DECLARE_THROW_SCOPE(state->vm());
    auto* thisObject = castForAttribute(*state, thisValue);
    if (UNLIKELY(!thisObject)) {
        if (shouldThrow == CastedThisErrorBehavior::Throw)
            return throwGetterTypeError(*state, throwScope, JSClass::info()->className, attributeName);
        if (shouldThrow == CastedThisErrorBehavior::RejectPromise)
            return rejectPromiseWithGetterTypeError(*state, JSClass::info()->className, attributeName);
        return JSC::JSValue::encode(JSC::jsUndefined());
    }
    return JSC::JSValue::encode(getter(*state, *thisObject, throwScope));
}

With castForAttribute(...) being implemented as:

template<> inline JSHTMLDocument* BindingCaller<JSHTMLDocument>::castForAttribute(ExecState& state, EncodedJSValue thisValue)
{
    return jsDynamicDowncast<JSHTMLDocument*>(state.vm(), JSValue::decode(thisValue));
}

If we manually inline and dead-code eliminate, it looks something like:

EncodedJSValue jsHTMLDocumentFgColor(ExecState* state, EncodedJSValue thisValue, PropertyName)
{
    auto throwScope = DECLARE_THROW_SCOPE(state->vm());
    auto* thisObject = jsDynamicDowncast<JSHTMLDocument*>(state.vm(), JSValue::decode(thisValue));
    if (UNLIKELY(!thisObject))
        return throwGetterTypeError(*state, throwScope, JSClass::info()->className, "fgColor");

    JSValue result = toJS<IDLTreatNullAsEmptyAdaptor<IDLDOMString>>(*state, thisObject->wrapped().fgColor());
    return JSC::JSValue::encode(result);
}

The first four lines are essentially the same for all DOM attributes with the following issues: 
- the class used for the jsDynamicDowncast is different per class 
- the error string will obviously be different, but is available via the PropertyName parameter as well ) and if you had access
- There are some attributes that don’t throw due to [LenientThis]

So, it seems like it should be plausible to make special variants of JSC::CustomGetterSetter and JSC::JSCustomGetterSetterFunction that know how to do that initial check. 

If we do that, the next logical step would be to teach the DFG about the prologue, but actually, that kind of already exists in the form of DOMJIT's CheckDOM (credit to Yusuke Suzuki for making this connection), so hopefully we could reuse that infrastructure. In the end, this would mean we could reduce the code size, and speedup attributes! NOTE: A similar optimization could probably be done for function prologues.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170504/67b96329/attachment.html>