842192 - Self-host Array.map

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

Attachment: v1

While the self-hosted version of Array.map isn't many times as fast as the native one, as was the case for Array.forEach, a simple test-case using the mapping function `function(item, idx, list) {return item * idx}` on a map with 10000000 entries went from 1110ms to 645ms for me.

Also: 2 files changed, 51 insertions(+), 87 deletions(-)

Comment 1

[Till Schneidereit [:till]]

Attachment: Simple benchmark

Simple benchmark, showing a speedup of about 2x.

Changing the UnsafeSetElement in Step c.iii to `A[i] = mappedValue` improves the speedup to about 7x.

Comment 2

[Till Schneidereit [:till]]

Comment on attachment 716519 [details]
Simple benchmark

After talking to nmatsakis on IRC, I updated to current tip.

With that, the self-hosted Array.map is now ~6.5x as fast as the native version: from 1110ms to 179ms.

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 714986 [details] [diff] [review]
v1

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

::: js/src/builtin/Array.js
@@ +384,5 @@
> +}
> +
> +function ArrayStaticMap(list, fun/*, receiver */) {
> +    if (arguments.length < 2)
> +        ThrowError(JSMSG_MISSING_FUN_ARG, 0, 'Array.map');

It would be consistent here to check whether fun is callable, as in ArrayStaticForEach and friends (in order to produce a better error message).

@@ +385,5 @@
> +
> +function ArrayStaticMap(list, fun/*, receiver */) {
> +    if (arguments.length < 2)
> +        ThrowError(JSMSG_MISSING_FUN_ARG, 0, 'Array.map');
> +    var receiver = arguments.length > 1 ? arguments[1] : void 0;

2, not 1, right?

@@ +386,5 @@
> +function ArrayStaticMap(list, fun/*, receiver */) {
> +    if (arguments.length < 2)
> +        ThrowError(JSMSG_MISSING_FUN_ARG, 0, 'Array.map');
> +    var receiver = arguments.length > 1 ? arguments[1] : void 0;
> +    callFunction(ArrayMap, list, fun, receiver);

Need a return statement.

::: js/src/jsarray.cpp
@@ +2491,5 @@
>  
>           {"lastIndexOf",        {NULL, NULL},       1,0, "ArrayLastIndexOf"},
>           {"indexOf",            {NULL, NULL},       1,0, "ArrayIndexOf"},
>           {"forEach",            {NULL, NULL},       1,0, "ArrayForEach"},
> +         {"map",                {NULL, NULL},       1,0, "ArrayMap"},

We need an entry for ArrayStaticMap in array_static_methods.

Comment 4

[Till Schneidereit [:till]]

Pushed with comments addressed and some naming fixes
https://hg.mozilla.org/integration/mozilla-inbound/rev/788c24847525

Comment 5

[Mozilla RelEng Bot]

Try run for 0a987a59e6b2 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=0a987a59e6b2
Results (out of 40 total builds):
    success: 38
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-0a987a59e6b2

Comment 6

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/788c24847525