1295379 - Improve performance of various TypedArray built-ins

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[André Bargull [:anba]]

I've identified two performance problems affecting the self-hosted TypedArray methods:
- Calling Array.prototype.push on List objects is slow.
- Functions which use default arguments and also access |arguments| are slow.


Micro benchmark:
---
function test(fn) {
    var t = 0;
    for (var i = 0; i < 1000; ++i) {
        var d = dateNow();
        fn();
        t += (dateNow() - d);
    }
    return t;
}

function testProto(fn) {
    var ta = new Int8Array(10000);
    return test(() => fn(ta));
}

function testFrom() {
    var elements = new Array(10000);
    elements.fill(0);
    return test(() => Int8Array.from(elements));
}

for (var i = 0; i < 10; ++i) {
    print(testFrom());
    // print(testProto(ta => ta.filter(() => true)));
    // print(testProto(ta => ta.map((k, v) => v)));
    // print(testProto(ta => ta.forEach(() => {})));
    // print(testProto(ta => ta.every(() => true)));
    // print(testProto(ta => ta.some(() => false)));
    // print(testProto(ta => ta.find(() => false)));
    // print(testProto(ta => ta.findIndex(() => false)));
}
---


TypedArray.from:
  before: ~2500 ms
  after: ~230 ms

TypedArray.prototype.filter:
  before: ~3450 ms
  after: ~180 ms

TypedArray.prototype.map:
  before: ~350 ms
  after: ~40 ms

TypedArray.prototype.{forEach, every, some, find, findIndex}:
  before: ~300 ms
  after: ~20 ms

Comment 1

[André Bargull [:anba]]

Attachment: Part 0 - Update TypedArray.from to match current draft spec

Comment 2

[André Bargull [:anba]]

Attachment: Part 1 - Workaround perf issues

This patch provides workarounds for the problems identified in comment #0. Fixing the underlying issues is probably harder and out of my league, but if it's okay to workaround the current performance weak spots, we could apply this patch until a proper fix is available.

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 8781309 [details] [diff] [review]
Part 1 - Workaround perf issues

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

Was just gonna driveby the next paragraph, then I started looking, might as well just stamp, this is simple.  :-)

<musings>
For what it's worth, explicitly having List as a thing is stupid and grody.  It was more or less fine when just Intl used it, because that needn't be super-fast and it was a first-cut implementation anyway.  But it's been awhile, and I guess actually-more-important stuff is now using it.  It's probably time to replace it with something real, tho I'm not sure what that should be.

V8 has intrinsics to get particular numeric arguments, rather than use |arguments|.  That lets them avoid some slowness that |arguments| might induce, such as what occurs when defaults/rests are present.  I would be in favor of having similar intrinsics in SpiderMonkey.
</musings>

Comment 4

[Jeff Walden [:Waldo]]

(To be clear, on the policy question of taking this versus fixing underlying issues, and f+ versus r+, I think we take this.  And as far as arguments+defaults go, my recollection of semantics is that once you have that, things *necessarily* get slower because you *have* to reify an arguments object as a general rule, because the two occupy separate storage.  So that problem possibly isn't fixable.)

Comment 5

[André Bargull [:anba]]

Comment on attachment 8781308 [details] [diff] [review]
Part 0 - Update TypedArray.from to match current draft spec

Remove part 0 - Updating the implementation to match the current spec should be done as part of bug 1122396.

Comment 6

[André Bargull [:anba]]

Attachment: Workaround perf issues

Updated patch so it longer depends on part 0, and also fixed the JS_SELF_HOSTED_FN length parameters in vm/TypedArrayObject.cpp. 

Carrying r+ from Waldo. (Someone needs to push the patch to Try for me, because I still haven't requested try-access...)

Comment 7

[André Bargull [:anba]]

Attachment: bug1295379.patch

Patch needed bit-unrotting, carrying r+ from Waldo.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d55e89b2945

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8af68ee1064
Avoid performance pitfalls in TypedArray built-ins. r=Waldo

Comment 9

[Phil Ringnalda (:philor)]

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

Comment 10

[Gerry Chang [:gchang]]

Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.