Closed Bug 1526315 Opened 7 months ago Closed 6 months ago

Non-integer typed array access can lead to repeated bailouts

Categories

(Core :: JavaScript Engine: JIT, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: anba, Assigned: whawkins)

References

Details

(Whiteboard: [qf:p2:responsiveness])

Attachments

(1 file)


function f() {
    var ta = new Int32Array(100000);
    var xs = [0, 1, 2, 59853.5];
    var r = 0;
    for (var i = 0; i < 100000; ++i) {
        r += ta[xs[i & 3]];
    }
    return r;
}
for (var i = 0; i < 10; ++i) print(f());

Prints repeated "[IonBailouts] Took bailout!" when running with IONFLAGS=bailouts.

This was the underlying cause of the performance issue reported in bug 1515620, see bug 1515620 comment #2.

Priority: -- → P2
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p3:responsiveness]
Whiteboard: [qf:p3:responsiveness] → [qf:p2:responsiveness]

I think that there is an opportunity to do something here. I am thinking about adding a fix for the case where, like here, all the indices are constants. In that case, if the constants are all statically, "cleanly" convertible to integers, we go ahead with the optimization. In the case where that's not true, we skip ionization entirely.

I'd love to hear what people thought about that approach.

It seems to be what already happens implicitly. In other words, if the index is a double but converts to an integer without loss of precision, the ionized code runs happily. On the other hand, when there is a loss of precision, there is a bailout.

comment #0 uses constant indices for the sake of simplicity, but the original issue described in bug 1515620 was using non-constant indices.

I wonder if we could handle non-integer indices more like OOB access, which also doesn't lead to repeated bailouts. But as a prerequisite for that, we'd need to update our TypedArray implementation to follow the spec more closely, because right now non-integer indices perform a prototype lookup whereas the spec actually requires us to omit the prototype lookup:

Object.prototype[0.5] = "hello";
print(new Int32Array()[0.5]);

shouldn't print "hello", but currently does in SpiderMonkey. -> bug 1129202

This speeds up the above "benchmark" by around 25%.

real 0m3.389s
user 0m3.383s
sys 0m0.028s

real 0m4.886s
user 0m6.998s
sys 0m0.110s

Those figures were gathered completely unscientifically.

JS shell with those changes applied passes regressions.

Keywords: checkin-needed

Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8098dde10d8
Non-integer typed array access can lead to repeated bailouts r=iain

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