Use StrictEq semantics for more cases in Object.is

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Bug 1433432 was a bit conservative when to convert Object.is() into JSOP_STRICTEQ, for example MCompare::Compare_Bitwise is currently not enabled.
Posted patch bug1482359.patch (obsolete) — Splinter Review
Use JSOP_STRICTEQ for more cases in Object.is():
- When both operands are typed as MIRType::Value, but are definitely not floating-point types, we can use JSOP_STRICTEQ.
- When one operand is typed as MIRType::Value, but also definitely not a floating-point type, we can use JSOP_STRICTEQ, too.

And when inlining Object.is() as JSOP_STRICTEQ, directly call |jsop_compare| instead of just |compareTrySpecialized| to be able to use MCompare::Compare_Bitwise via |compareTryBitwise|.


Improves test cases like this one from 240ms to 28ms for me:
---
function f() {
    var xs = [1,2,3,null];
    var ys = [1,2,3,null];
    var q = 0;
    var t = dateNow();
    for (var i = 0; i < 10000000; ++i) {
        if (Object.is(xs[i & 3], ys[i & 3]))
            ++q;
    }
    print(dateNow() - t, q);
}
f();
---
Attachment #8999149 - Flags: review?(jdemooij)
Comment on attachment 8999149 [details] [diff] [review]
bug1482359.patch

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

Nice speedup and I like reusing more/most of the jsop_compare code.

::: js/src/jit/MCallOptimize.cpp
@@ +2499,5 @@
> +        // they aren't floating-point types or values which might be floating-
> +        // point types. Otherwise we need to use MSameValue.
> +        strictEq = leftType != MIRType::Value
> +                   ? !IsFloatingPointType(leftType)
> +                   : !mightBeFloatingPointType(left) && !mightBeFloatingPointType(right);

Nit: I would add parentheses around the && because I never remember the evaluation order in such cases and I'm too lazy to look it up :)
Attachment #8999149 - Flags: review?(jdemooij) → review+
Posted patch bug1482359.patch (obsolete) — Splinter Review
Updated per review comments, carrying r+.
Attachment #8999149 - Attachment is obsolete: true
Attachment #8999602 - Flags: review+
Clearing "checkin-needed" because the patch needs to be rebased now that bug 1341261 has landed.
Keywords: checkin-needed
Rebased patch to apply cleanly on inbound.
Attachment #8999602 - Attachment is obsolete: true
Attachment #8999732 - Flags: review+
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a23c3d37ae2b
Use more JSOP_STRICTEQ optimizations for Object.is(). r=jandem
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a23c3d37ae2b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.