1398751 - Add a fast path for typed arrays to js::GetElements

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[André Bargull [:anba]]

This will lead to almost exactly the same speed-ups as in V8 (https://bugs.chromium.org/p/v8/issues/detail?id=2435), i.e. 2-3x faster on the µ-benchmark in https://bugs.chromium.org/p/v8/issues/detail?id=2435#c6.

Comment 1

[André Bargull [:anba]]

Attachment: bug1398751-part1-getelements-for-typedarrays.patch

This improves the benchmark in https://bugs.chromium.org/p/v8/issues/detail?id=2435#c6 (with |n| changed to 1e4 instead of 1e5, because my PC is slower and I'm testing in a VM :-) ) as follows:

stringFromCharCode: 985ms -> 305ms
stringFromCodePoint: 1210ms -> 515ms

Before the V8 patch, V8 and SM were both slower than JSC/Chakra, now that V8 is patched, we're the slowest engine for that µ-benchmark. But with this patch, we're faster than JSC/Chakra and I guess we get comparable numbers to V8 again, but since I haven't yet recompiled V8, I can't tell for sure.

Comment 2

[André Bargull [:anba]]

Attachment: bug1398751-part2-codepoint-from-int32.patch

And this gives a further improvement for the stringFromCodePoint case:

With patch 1:   1210ms -> 515ms
With patch 1+2: 1210ms -> 335ms

Comment 3

[Tom S [:evilpie]]

Comment on attachment 8906606 [details] [diff] [review]
bug1398751-part1-getelements-for-typedarrays.patch

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

Nice, very clean.

::: js/src/jsarray.cpp
@@ +443,5 @@
> +    MOZ_ASSERT(length == typedArray->length());
> +    MOZ_ASSERT_IF(length > 0, !typedArray->hasDetachedBuffer());
> +
> +    for (uint32_t i = 0; i < length; ++i, ++vp) {
> +        *vp = typedArray->getElement(i);

I hope compilers are clever enough to do loop unswitching here and we don't check the type in every iteration.

Comment 4

[Tom S [:evilpie]]

Comment on attachment 8906607 [details] [diff] [review]
bug1398751-part2-codepoint-from-int32.patch

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

Wow this is a big difference.

::: js/src/jsstr.cpp
@@ +3315,5 @@
> +
> +    // Fast path for the common case - the input is already an int32.
> +    if (code.isInt32()) {
> +        int32_t nextCP = code.toInt32();
> +        if (!(nextCP < 0 || nextCP > int32_t(unicode::NonBMPMax))) {

I think our usual style is to invert the conditions.

Comment 5

[André Bargull [:anba]]

(In reply to Limited Availabilty till Oct | Tom Schuster [:evilpie] from comment #3)
> I hope compilers are clever enough to do loop unswitching here and we don't
> check the type in every iteration.

Good remark! I just tested this and at least GCC 5.4 doesn't seem to hoist the type check out of the loop. Manually switching the loop and the switch-statement further improved the µ-benchmark to

stringFromCharCode: 215ms
stringFromCodePoint: 240ms

Comment 6

[André Bargull [:anba]]

Attachment: bug1398751-part1-new-getelements-for-typedarrays.patch

New patch for part 1, because compilers don't seem to perform loop unswitching. Reduces the time needed to complete the benchmark by another 90ms when compared to the previous patch.

Comment 7

[Tom S [:evilpie]]

Comment on attachment 8906747 [details] [diff] [review]
bug1398751-part1-new-getelements-for-typedarrays.patch

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

Great

Comment 8

[André Bargull [:anba]]

Attachment: bug1398751-part2-codepoint-from-int32.patch

Invert condition in if-statement as suggested in review comment. Carrying r+ from :evilpie.

Comment 9

[André Bargull [:anba]]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbf2ccf813e
Part 1: Add fast-path for typed arrays in js::GetElements to speed-up Function.apply with typed arrays. r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0f98cfb1a45
Part 2: Add fast path to convert int32 value to a code point. r=evilpie

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fdbf2ccf813e
https://hg.mozilla.org/mozilla-central/rev/f0f98cfb1a45