1526315 - Non-integer typed array access can lead to repeated bailouts

Status: RESOLVED FIXED

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

Description

[André Bargull [:anba]]

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.

Comment 1

[Will Hawkins]

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 2

[André Bargull [:anba]]

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

Comment 3

[Will Hawkins]

Attachment: Bug 1526315: Non-integer typed array access can lead to repeated bailouts

Comment 4

[Will Hawkins]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8098dde10d8
Non-integer typed array access can lead to repeated bailouts r=iain

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d8098dde10d8