Closed Bug 1046197 Opened 6 years ago Closed 6 years ago

Make ArrayIteratorNext and StringIteratorNext easier to optimized with Scalar Replacement.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file)

The current implementation of scalar replacement does not accept to have a Phi which is used to merge 4 MNewObject, even if the 4 of them are non-escaped objects in practice.  The following micro benchmark is hitting this case with the inlining of ArrayIteratorNext.

    function f(arr) {
        var t = new Date;
        var sum = 0;
        //for (var i=0; i<100000; i++) {
            var iter = arr["@@iterator"]();
            while (true) {
                var obj = iter.next();
                if (obj.done) {
                    obj = undefined;
                    break;
                }
                sum += obj.value;
                obj = undefined;
            }
        //}
        print(new Date - t);
        print(sum);
    }
     
    var a = [];
    for (var i=0; i<10000000; i++)
        a.push(i % 3);
    f(a);

To handle this properly in the Scalar replacement phase we would have to check that all objects following into the Phi are non-escaped and have the same shape as well as checking that the Phi is not escaped either.  This is what an escape analysis should do, but our current conservative one is too trivial to detect such cases and it is easier to tweak our builtins to fit the optimizable pattern.
Attachment #8464759 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/48f6eb406821
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.