Closed
Bug 781387
Opened 13 years ago
Closed 13 years ago
Paris Bindings: Codegen infalliblity for methods
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: efaust, Assigned: efaust)
References
Details
Attachments
(1 file)
5.66 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
Infallible methods with infallibly boxed return types and no arguments can be marked infallible for further JIT optimization.
Assignee | ||
Comment 1•13 years ago
|
||
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.
Attachment #650415 -
Flags: review?(peterv)
Assignee | ||
Comment 2•13 years ago
|
||
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;
Attachment #650415 -
Attachment is patch: true
Comment 3•13 years ago
|
||
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
Assignee | ||
Comment 4•13 years ago
|
||
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.
Updated•13 years ago
|
Attachment #650415 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 5•13 years ago
|
||
Comment 6•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•