781387 - Paris Bindings: Codegen infalliblity for methods

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Eric Faust [:efaust]]

Infallible methods with infallibly boxed return types and no arguments can be marked infallible for further JIT optimization.

Comment 1

[Eric Faust [:efaust]]

Attachment: Patch

Performance improves by maybe 6-9%, dropping timings on the trivial microbenchmark calling CanvasRenderingContext2d.restore() in a tight loop under the jit from about 103ms to 94ms or so. 

Unclear how much we care to do this, as it only helps us in very simplistic cases.

Comment 2

[Eric Faust [:efaust]]

Comment on attachment 650415 [details] [diff] [review]
Patch

>diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py
>--- a/dom/bindings/Codegen.py
>+++ b/dom/bindings/Codegen.py
>@@ -2633,29 +2633,29 @@ def wrapForType(type, descriptorProvider
>     wrap = getWrapTemplateForType(type, descriptorProvider,
>                                   templateValues.get('result', 'result'),
>                                   templateValues.get('successCode', None),
>                                   templateValues.get('isCreator', False))[0]
> 
>     defaultValues = {'obj': 'obj'}
>     return string.Template(wrap).substitute(defaultValues, **templateValues)
> 
>-def infallibleForAttr(attr, descriptorProvider):
>+def infallibleForMember(member, type, descriptorProvider):
>     """
>     Determine the fallibility of changing a C++ value of IDL type "type" into
>     JS for the given attribute. Apart from isCreator, all the defaults are used,
>     since the fallbility does not change based on the boolean values,
>     and the template will be discarded.
> 
>     CURRENT ASSUMPTIONS:
>         We assume that successCode for wrapping up return values cannot contain
>         failure conditions.
>     """
>-    return getWrapTemplateForType(attr.type, descriptorProvider, 'result', None,\
>-                                  memberIsCreator(attr))[1]
>+    return getWrapTemplateForType(type, descriptorProvider, 'result', None,\
>+                                  memberIsCreator(member))[1]
> 
> def typeNeedsCx(type):
>     return (type is not None and
>             (type.isCallback() or type.isAny() or type.isObject() or
>              (type.isUnion() and
>               any(typeNeedsCx(t) for t in type.unroll().flatMemberTypes))))
> 
> # Returns a CGThing containing the type of the return value, or None
>@@ -3364,39 +3364,52 @@ class CGMemberJITInfo(CGThing):
>         protoID = "prototypes::id::%s" % self.descriptor.interface.identifier.name
>         depth = "PrototypeTraits<%s>::Depth" % protoID
>         failstr = "true" if infallible else "false"
>         return ("\n"
>                 "const JSJitInfo %s = {\n"
>                 "  %s,\n"
>                 "  %s,\n"
>                 "  %s,\n"
>-                "  %s,  /* isInfallible. Only relevant for getters. */\n"
>+                "  %s,  /* isInfallible. False in setters. */\n"
>                 "  false  /* isConstant. Only relevant for getters. */\n"
>                 "};\n" % (infoName, opName, protoID, depth, failstr))
> 
>     def define(self):
>         if self.member.isAttr():
>             getterinfo = ("%s_getterinfo" % self.member.identifier.name)
>             getter = ("(JSJitPropertyOp)get_%s" % self.member.identifier.name)
>             getterinfal = "infallible" in self.descriptor.getExtendedAttributes(self.member, getter=True)
>-            getterinfal = getterinfal and infallibleForAttr(self.member, self.descriptor)
>+            getterinfal = getterinfal and infallibleForMember(self.member, self.member.type, self.descriptor)
>             result = self.defineJitInfo(getterinfo, getter, getterinfal)
>             if not self.member.readonly:
>                 setterinfo = ("%s_setterinfo" % self.member.identifier.name)
>                 setter = ("(JSJitPropertyOp)set_%s" % self.member.identifier.name)
>                 # Setters are always fallible, since they have to do a typed unwrap.
>                 result += self.defineJitInfo(setterinfo, setter, False)
>             return result
>         if self.member.isMethod():
>             methodinfo = ("%s_methodinfo" % self.member.identifier.name)
>             # Actually a JSJitMethodOp, but JSJitPropertyOp by struct definition.
>             method = ("(JSJitPropertyOp)%s" % self.member.identifier.name)
>-            # Method, much like setters, are always fallible
>-            result = self.defineJitInfo(methodinfo, method, False)
>+            
>+            # Methods are infallible if they are infallbile, have no arguments
>+            # to unwrap, and have a return type that's infallbile to wrap up for
>+            # return.
>+            methodInfal = False
>+            sigs = self.member.signatures()
>+            if len(sigs) == 1:
>+                # Don't handle overloading. If there's more than one signature,
>+                # one of them must take arguments.
>+                sig = sigs[0]
>+                if len(sig[1]) == 0 and infallibleForMember(self.member, sig[0], self.descriptor):
>+                    # No arguments and infallible return boxing
>+                    methodInfal = True
>+
>+            result = self.defineJitInfo(methodinfo, method, methodInfal)
>             return result
>         raise TypeError("Illegal member type to CGPropertyJITInfo")
> 
> def getEnumValueName(value):
>     # Some enum values can be empty strings.  Others might have weird
>     # characters in them.  Deal with the former by returning "_empty",
>     # deal with possible name collisions from that by throwing if the
>     # enum value is actually "_empty", and throw on any value
>diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h
>--- a/js/src/jsfriendapi.h
>+++ b/js/src/jsfriendapi.h
>@@ -1331,17 +1331,17 @@ typedef bool
> typedef bool
> (* JSJitMethodOp)(JSContext *cx, JSHandleObject thisObj,
>                   void *specializedThis, unsigned argc, JS::Value *vp);
> 
> struct JSJitInfo {
>     JSJitPropertyOp op;
>     uint32_t protoID;
>     uint32_t depth;
>-    bool isInfallible;    /* Is op fallible? Getters only */
>+    bool isInfallible;    /* Is op fallible? False in setters. */
>     bool isConstant;      /* Getting a construction-time constant? */
> };
> 
> static JS_ALWAYS_INLINE const JSJitInfo *
> FUNCTION_VALUE_TO_JITINFO(const JS::Value& v)
> {
>     JS_ASSERT(js::GetObjectClass(&v.toObject()) == &js::FunctionClass);
>     return reinterpret_cast<js::shadow::Function *>(&v.toObject())->jitinfo;

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 650415 [details] [diff] [review]
Patch

Review of attachment 650415 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +3389,5 @@
>          if self.member.isMethod():
>              methodinfo = ("%s_methodinfo" % self.member.identifier.name)
>              # Actually a JSJitMethodOp, but JSJitPropertyOp by struct definition.
>              method = ("(JSJitPropertyOp)%s" % self.member.identifier.name)
> +            

Trailing whitespace

@@ +3390,5 @@
>              methodinfo = ("%s_methodinfo" % self.member.identifier.name)
>              # Actually a JSJitMethodOp, but JSJitPropertyOp by struct definition.
>              method = ("(JSJitPropertyOp)%s" % self.member.identifier.name)
> +            
> +            # Methods are infallible if they are infallbile, have no arguments

infallible

@@ +3391,5 @@
>              # Actually a JSJitMethodOp, but JSJitPropertyOp by struct definition.
>              method = ("(JSJitPropertyOp)%s" % self.member.identifier.name)
> +            
> +            # Methods are infallible if they are infallbile, have no arguments
> +            # to unwrap, and have a return type that's infallbile to wrap up for

And here

Comment 4

[Eric Faust [:efaust]]

With IonMonkey to land, in TBPL parlance "Any Minute Now", this would be cool to actually finish up. It does represent about a 6% performance win.

Comment 5

[Eric Faust [:efaust]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e35d65f622a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e35d65f622a