1376691 - Inline Array.isArray properly in Ion

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Jan de Mooij [:jandem]]

Attachment: Patch

I noticed Speedometer's AngularJS test was calling Array.isArray about 400,000 times. Most of these calls are from Ion because we failed to inline them.

Ion can fold/nop Array.isArray in some cases, but this is pretty brittle. I'm attaching a patch to make isArray inlining a lot more robust - worst case we now use MIsArray which is still pretty fast and has an OOL call for proxies.

A few random micro-benchmarks to demonstrate this:

* testValues tests the case where the input is either an object or primitive and we use MIsArray. This is pretty common in the wild and this microbench improves from 320 ms -> 122 ms.

* testNonArrayObjects is when TI knows statically that all objects are definitely not arrays. We handle this more robustly now: 346 ms -> 41 ms.

Speedometer's AngularJS test also improves a bit but it's pretty noisy so let's see what AWFY says.

function testValues() {
    var arr = [1, null, [], {}];
    var res = 0;
    var t = new Date;
    for (var i = 0; i < 10000000; i++) {
        for (var j = 0; j < 4; j++) {
            if (Array.isArray(arr[j]))
                res++;
        }
    }
    print(new Date - t);
    return res;
}
testValues();

function testNonArrayObjects() {
    var arr = [{}, {}, Math, this];
    var res = 0;
    var t = new Date;
    for (var i = 0; i < 10000000; i++) {
        for (var j = 0; j < 4; j++) {
            if (Array.isArray(arr[j]))
                res++;
        }
    }
    print(new Date - t);
    return res;
}
testNonArrayObjects();

Comment 1

[Nicolas B. Pierron [:nbp]]

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

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

Good catch!

::: js/src/jit/MCallOptimize.cpp
@@ +600,3 @@
>      }
>  
> +    using ForAllResult = TemporaryTypeSet::ForAllResult;

nit: using TemporaryTypeSet::ForAllResult;

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> > +    using ForAllResult = TemporaryTypeSet::ForAllResult;
> 
> nit: using TemporaryTypeSet::ForAllResult;

I just tried that one, but the compiler doesn't like it:

MCallOptimize.cpp:602:29: error: using declaration cannot refer to class member
    using TemporaryTypeSet::ForAllResult;
          ~~~~~~~~~~~~~~~~~~^
MCallOptimize.cpp:602:11: note: use an alias declaration instead
    using TemporaryTypeSet::ForAllResult;
          ^
          ForAllResult =

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef43b448dab
Improve Ion-inlining of Array.isArray. r=nbp

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ef43b448dab