749866 - Improve overload resolution to avoid double-testing as much as possible

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Right now overload resolution does a lot of "if this is a Foo object and we have an overload that wants Bar then generate code that unwraps Foo to Bar" kinds of things, but in practice most of those cases in the unwrapping to Bar end up rechecking that we have a Foo object.  A better setup would be to just try unwrapping to Bar and if that fails to move on to the next overload.  That's what we already do for interface types and will do for typed arrays in bug 749864; I think we can also do it for sequences at least.  We just have to make sure our cases are ordered correctly so we comply with the spec.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I should probably wait until Peter finishes up bug 756258, since chances are the same code is involved.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Improvement in XHR codegen

This improvement comes from skipping the extra null-or-undefined codegen for primitives/strings/enums, since we'll end up in their trailing case anyway after skipping all the object types because we're not an object.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Improvement in WebGLRenderingContext codegen

This is from the thing the bug was filed on originally: not double-testing the IsArrayLike() bit.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add support for failureCode for sequence unwrapping.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Simplify the code generated by overload resolution a bit when we have sequences or dates at our distinguishing index.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: More improvement in WebGLRenderingContext codegen, this time for nullable object distinguishing args

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Rejigger overload resolution a bit more so we can tell argument conversion that we know the argument is null or undefined.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Generate slightly better code when we know we're null or undefined.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 672647 [details] [diff] [review]
part 2.  Simplify the code generated by overload resolution a bit when we have sequences or dates at our distinguishing index.

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

::: dom/bindings/Codegen.py
@@ +3376,5 @@
> +            # We should not have "any" args at distinguishingIndex,
> +            # since we have multiple possible signatures remaining,
> +            # but "any" is never distinguishable from anything else.
> +            for (_, args) in possibleSignatures:
> +                assert not args[distinguishingIndex].type.isAny()

Why not combine this check and the previous check into the same loop?

@@ +3413,5 @@
> +            # First check for null or undefined.  But ignore nullable
> +            # primitives, strings, and enums here, because if we actually have
> +            # null or undefined it won't unwrap to any of the object arguments
> +            # anyway, so we'll automatically end up in the trailing
> +            # primitive/string/enum case.

A little more explanation (that all object stuff is guarded on isObject, and only one type can be nullable) would be useful here, I think.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Why not combine this check and the previous check into the same loop?

Done.

> A little more explanation 

Comment now says:

            # First check for null or undefined.  That means looking for
            # nullable arguments at the distinguishing index and outputting a
            # separate branch for them.  But if the nullable argument is a
            # primitive, string, or enum, we don't need to do that.  The reason
            # for that is that at most one argument at the distinguishing index
            # is nullable (since two nullable arguments are not
            # distinguishable), and all the argument types other than
            # primitive/string/enum end up inside isObject() checks.  So if our
            # nullable is a primitive/string/enum it's safe to not output the
            # extra branch: we'll fall through to conversion for those types,
            # which correctly handles null as needed, because isObject() will be
            # false for null and undefined.

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b02a816de33
https://hg.mozilla.org/integration/mozilla-inbound/rev/024d12cc4009
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b2c87bfe082
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f08c574fb79

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0b02a816de33
https://hg.mozilla.org/mozilla-central/rev/024d12cc4009
https://hg.mozilla.org/mozilla-central/rev/5b2c87bfe082
https://hg.mozilla.org/mozilla-central/rev/6f08c574fb79