Closed Bug 1523642 Opened 10 months ago Closed 9 months ago

Improve generated code for Array.prototype.reduce[Right]

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(5 files)

Some slight modifications for Array.prototype.reduce and Array.prototype.reduceRight will allow Ion to generate better code. The attached patch improves the following test case:

  • Four distinct arrays: 250ms -> 120ms
  • Eight distinct arrays: 500ms -> 380ms (exceeds TYPE_FLAG_OBJECT_COUNT_LIMIT, somewhat similar to bug 1520251)

Test case:

function f() {
    var a = [
        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],

        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],
        [0,0,0, 0,0,0, 0,0,0],
    ];
    var r = 0;
    var t = dateNow()
    for (var i = 0; i < 4000000; ++i) {
        r += a[i & 7].reduce((a, c) => a + c);
    }
    return [dateNow() - t, r];
}
for (var i = 0; i < 10; ++i) print(f());

Also see attached IonGraph output

I've used the following modified test case for generating the IonGraph output (basically only the array declaration and timing code was removed):

do {
// Declare in loop to avoid assigning singleton types.
var a = [
    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],

    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],
    [0,0,0, 0,0,0, 0,0,0],
];
} while(false);

function f() {
    var r = 0;
    for (var i = 0; i < 10000; ++i) {
        r += a[i & 3].reduce((a, c) => a + c);
    }
    return r;
}
for (var i = 0; i < 2; ++i) print(f(a));
Attached patch bug1523642.patchSplinter Review

Changes:

  • Add throw to ThrowTypeError calls so that Ion knows the function doesn't continue after ThrowTypeError. (I'll file a follow-up bug to change the BytecodeEmitter to do this automatically.)
  • Remove the IsPackedArray call, which was added before the in-operator was better optimised. Today calling IsPackedArray doesn't help or actually hurts performance if we're not able to inline it (bug 1383643).
  • Change a for-loop into a do/while-loop, so that Ion can see that the loop is definitely entered at least once, which then enables Ion to remove the complete loop in some cases (see "four-elements-after.pdf").
  • Move the assignment of accumulator value out of the loop, so that Ion doesn't add undefined (the initial, implicit value of accumulator) to the type-set of accumulator. This avoids some extra boxing, see the attached PDFs.
Attachment #9039829 - Flags: review?(tcampbell)
Comment on attachment 9039829 [details] [diff] [review]
bug1523642.patch

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

Sorry for the huge delay. This is quality work! A+
Attachment #9039829 - Flags: review?(tcampbell) → review+

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a58c63890cc
Improve Ion-generated code for Array.prototype.reduce[Right]. r=tcampbell

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.