1165052 - Implement Array[@@species] and ArraySpeciesCreate in Array prototype methods.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

derived from bug 1131043.

Array[@@species] is used by ArraySpeciesCreate, which is used in following methods. we should add them at the same time, for feature test.
  Array.prototype.concat
  Array.prototype.filter
  Array.prototype.map
  Array.prototype.slice
  Array.prototype.splice

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-get-array-@@species
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-arrayspeciescreate
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.concat
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.filter
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.map
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.slice
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.splice

Comment 1

[Tooru Fujisawa [:arai]]

fwiw, here's WIP patches
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e5b09d44840

I think those shouldn't be landed before bug 1141863, since they introduce performance regression but has almost no merit without subclassing.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: array-species.zip

Here's WIP patches.
Will post performance comparison in next comment.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Performance comparison.

Here's performance comparison with today's m-c.
Now Array#filter is self-hosted in m-c (bug 1193280), so this patch just makes it slow :P
Performance regression in map/filter (both self-hosted) in Ion-execution is significant.  inlining ArraySpeciesCreate self-hosted function may help.

Comment 5

[Tooru Fujisawa [:arai]]

s/ArraySpeciesCreate self-hosted function/ArraySpeciesCreate function/

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: WIP patches

Rebased, and improved performance in map/filter
now ArraySpeciesCreate is implemented both with native and self-hosted.
Will post performance comparison in next comment.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Performance comparison.

Comment 8

[Tooru Fujisawa [:arai]]

Current patch is buggy with subclassing because of bug 1055472 comment #51.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Implement Array[@@species] getter.

Prepared 12 patches

[Part 1]

For now, concat, slice, and splice are native function, so @@species getter is also native function.

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Implement native ArraySpeciesCreate.

[Part 2]

For performance reason, ArraySpeciesCreate is implemented both in native and selfhosted.

Native ArraySpeciesCreate is implemented with 2 functions
  * GetNonDefaultArraySpecies
    gets @@species value from given object's constructor.
    returns undefined if @@species value is Array, or really undefined,
    otherwise returns @@species value
    undefined means we can create an array without invoking the @@species value,
    and optimize more in caller

  * ArraySpeciesCreateWith
    construct an array (or something) with non-default @@species value
    it should be called only if GetNonDefaultArraySpecies returns non-undefined value

Added some helper functions
  * IsWrappedArrayConstructor
    checks if passed argument is an Array constructor from another global
    in most case this should return false

  * IsArraySpecies
    checks if passes argument's constructor's @@species is Array

  * GetNativeGetterPure
    it gets getter function without invoking it,
    and returns only if it's native function.
    so that we can check if it's origianl @@species getter.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Implement self-hosted ArraySpeciesCreate.

[Part 3]

Self-hosted ArraySpeciesCreate does not call native ArraySpeciesCreate, to inline it in JIT.

IsArray, IsConstructor, and IsWrappedArrayConstructor are inlined in latter parts.

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Part 4: Inline IsArray intrinsic.

[Part 4]

IsArray can be replaced with Array.isArray, so it can be inlined with ArrayIsArray.

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Part 5: Inline IsConstructor intrinsic.

[Part 5]

IsConstructor can be inlined in almost same way as IsCallable, so the added code is similar to IsCallable's one

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Part 6: Inline IsWrappedArrayConstructor intrinsic.

[Part 6]

IsWrappedArrayConstructor is replaced with a constant |false| when the argument is not a Proxy, so that it cannot be a wrapped thing.
If the passes argument is an Array comes from same global, this should always be inlined and return false.  Other cases are not inlined.

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Part 7: Use ArraySpeciesCreate in Array.prototype.concat.

[Part 7]

Added ArraySpeciesCreate to Array.prototype.concat
  * ArrayConcatDense
    call array_concat_dense only if |this| value's constructor's @@species is Array

  * array_concat
    check @@species, and apply existing code only when @@species is Array
    otherwise create new Array with @@species value, and add each element with DefineElement

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Part 8: Use ArraySpeciesCreate in Array.prototype.filter.

[Part 8]

Just changed ArrayFilter to call ArraySpeciesCreate instead of [].

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Part 9: Use ArraySpeciesCreate in Array.prototype.map.

[Part 9]

Just changed ArrayMap to call ArraySpeciesCreate instead of [], and update comments.

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Part 10: Use ArraySpeciesCreate in Array.prototype.slice.

[Part 10]

Added ArraySpeciesCreate to Array.prototype.slice
  * ArraySliceOrdinary
    Split existing code for native Array from js::array_slice

  * js::array_slice
    call ArraySliceOrdinary only if @@species is Array
    otherwise create new Array with @@species value, and add each element with DefineElement

  * js::array_slice_dense
    use fast path only if |this| value's constructor's @@species is Array

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Part 11: Use ArraySpeciesCreate in Array.prototype.splice.

[Part 11]

Added ArraySpeciesCreate to Array.prototype.splice
  * ArraySpliceCopy
    Split copying code from js::array_splice_impl, that is used both in optimized (native Array) path and modified @@species path

  * js::array_splice_impl
    use existing path only if @@species is Array
    otherwise create new Array with @@species value

splice does not have *_dense variant for JIT

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Part 12: Add tests for ArraySpeciesCreate.

[Part 12]

Test for several @@species and constructor values, for each methods.

Comment 22

[Eric Faust [:efaust]]

Comment on attachment 8696158 [details] [diff] [review]
Part 1: Implement Array[@@species] getter.

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

Yep.

Comment 23

[Eric Faust [:efaust]]

Comment on attachment 8696159 [details] [diff] [review]
Part 2: Implement native ArraySpeciesCreate.

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

r=me with comments added and others addressed.

::: js/src/jsarray.cpp
@@ +884,5 @@
> +    // This must only return true if v is *the* Array constructor for the
> +    // current compartment; we rely on the fact that any other Array
> +    // constructor would be represented as a wrapper.
> +    return v.isObject() &&
> +           v.toObject().is<JSFunction>() &&

prefer |&& IsArrayConstructor(&v.toObject())|. That way we have one source of truth, and not duplicated entries.

@@ +904,5 @@
> +    if (!v.isObject()) {
> +        *result = false;
> +        return true;
> +    }
> +    if (v.toObject().is<CrossCompartmentWrapperObject>()) {

is<WrapperObject>(), I think should do.

@@ +937,5 @@
> +
> +    return getter == array_species_getter;
> +}
> +
> +/* ES6 9.4.2.3 steps 1-6. */

Unfortunately, the spec syntax changed, removing a step. Can you update the step numbers to match http://tc39.github.io/ecma262/#sec-arrayspeciescreate ?

@@ +953,5 @@
> +        pctor.setUndefined();
> +        return true;
> +    }
> +
> +    if (IsArraySpecies(cx, origArray)) {

This needs a comment, for sure. The idea is that if someone does something like:

{ constructor: Array }

that we can short-circuit, right?

@@ +989,5 @@
> +            pctor.setUndefined();
> +            return true;
> +        }
> +
> +        if (IsArrayConstructor(ctor)) {

Cute, to avoid calling it outright, but we need a comment here, also.

@@ +1021,5 @@
> +    ConstructArgs cargs(cx);
> +    if (!cargs.init(1))
> +        return false;
> +    if (length <= INT32_MAX)
> +        cargs[0].setInt32(length);

every last ounce of perf :P

Comment 24

[Eric Faust [:efaust]]

Comment on attachment 8696160 [details] [diff] [review]
Part 3: Implement self-hosted ArraySpeciesCreate.

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

Cancelling review here, despite wanting to keep this more than part 2. Please rerequest review for both when we come up with something.

::: js/src/builtin/Array.js
@@ +2,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +/* ES6 9.4.2.3. */
> +function ArraySpeciesCreate(originalArray, length) {

We talked on IRC about why this is here for performance reasons, but I don't think we can maintain two copies of the algorithm.

Let's try keeping just the self-hosted one, and seeing if we can make that work via one method or another.

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Performance comparison of slice/splice/concat.

for now, tested 2 cases.

* self-host all methods
  just self-host slice/splice/concat

* call self-hosted ArraySpeciesCreate from native slice/splice/concat
  check IsArraySpecies (instead of GetNonDefaultArraySpecies in previous patch),
  and if it's false, call self-hosted ArraySpeciesCreate from native slice/splice/concat
  in most normal case, ArraySpeciesCreate won't be called (IsArraySpecies returns true for array from same global)

slice and concat could be self-hosted, as gradient in Ion execution is better than or almost equal to m-c and previous patch.  there is some offset (< 2ms), but it will be ignorable.

On the other hand, self-hosted splice is slow in some case with straight-forward implementation.  we might need optimized path (maybe wrapping current native impl) if we need self-hosting it.

Easiest solution will be self-host slice and concat, and keep splice native, with calling IsArraySpecies and self-hosted ArraySpeciesCreate there.
Do you think it's okay to keep IsArraySpecies?

(other option will be wrap splice with self-hosted function, as I said in IRC, if above solution is not acceptable, I'll try implementing it ;)

Comment 26

[Jan de Mooij [:jandem]]

(In reply to Tooru Fujisawa [:arai] from comment #25)
> slice and concat could be self-hosted

FWIW, I've been thinking self-hosted Array#concat would be nice to have for Symbol.isConcatSpreadable as well..

Comment 27

[Eric Faust [:efaust]]

(In reply to Tooru Fujisawa [:arai] from comment #25)
> Created attachment 8699450 [details]
> Performance comparison of slice/splice/concat.
> 
> for now, tested 2 cases.
> 
> * self-host all methods
>   just self-host slice/splice/concat
> 
> * call self-hosted ArraySpeciesCreate from native slice/splice/concat
>   check IsArraySpecies (instead of GetNonDefaultArraySpecies in previous
> patch),
>   and if it's false, call self-hosted ArraySpeciesCreate from native
> slice/splice/concat
>   in most normal case, ArraySpeciesCreate won't be called (IsArraySpecies
> returns true for array from same global)
> 
> slice and concat could be self-hosted, as gradient in Ion execution is
> better than or almost equal to m-c and previous patch.  there is some offset
> (< 2ms), but it will be ignorable.
> 
> On the other hand, self-hosted splice is slow in some case with
> straight-forward implementation.  we might need optimized path (maybe
> wrapping current native impl) if we need self-hosting it.
> 
> Easiest solution will be self-host slice and concat, and keep splice native,
> with calling IsArraySpecies and self-hosted ArraySpeciesCreate there.
> Do you think it's okay to keep IsArraySpecies?
> 
> (other option will be wrap splice with self-hosted function, as I said in
> IRC, if above solution is not acceptable, I'll try implementing it ;)

This sounds like a good plan to me, and if there is call to self-host the other two functions, then we do good along the way :)

Comment 28

[Tooru Fujisawa [:arai]]

Thanks :)

Just found a bug for self-hosting splice (bug 890329)

Comment 29

[Tooru Fujisawa [:arai]]

Comment on attachment 8696161 [details] [diff] [review]
Part 4: Inline IsArray intrinsic.

clearing r? for now.

Comment 30

[Tooru Fujisawa [:arai]]

here's current plan:
  1. self-hosted Array#concat in bug 1233642
  2. keep Array#slice and Array#splice native functions
  3. implement ArraySpeciesCreate only in self-hosted JS
  4. call self-hosted ArraySpeciesCreate from Array#slice and Array#splice
     only if IsArraySpecies is false

Comment 31

[Tooru Fujisawa [:arai]]

here's WIP patch based on bug 1233642.

Comment 32

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Implement Array[@@species] getter.

Now @@species is called on self-hosted code, so moved it to self-hosted code.

Comment 33

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Add IsArraySpecies.

slice and splice are still native functions,
IsArraySpecies is used to avoid calling self-hosted ArraySpeciesCreate function.

GetGetterPure will be added in bug 1165053

Comment 34

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Add IsWrappedArrayConstructor intrinsic.

IsWrappedArrayConstructor is used to check ES 2016 spec 9.4.2.3 steps 5.b.1-iii.

https://tc39.github.io/ecma262/#sec-arrayspeciescreate

Comment 35

[Tooru Fujisawa [:arai]]

Attachment: Part 4: Inline IsConstructor intrinsic.

Almost same code as IsCallable.

Comment 36

[Tooru Fujisawa [:arai]]

Attachment: Part 5: Inline IsWrappedArrayConstructor intrinsic.

Comment 37

[Tooru Fujisawa [:arai]]

Attachment: Part 6: Implement self-hosted ArraySpeciesCreate.

now ArraySpeciesCreate is implemented only in self-hosted code.

Comment 38

[Tooru Fujisawa [:arai]]

Attachment: Part 8: Use ArraySpeciesCreate in Array.prototype.concat.

based on self-hosted A.p.concat in bug 1233642.
it's now so simple.

Comment 39

[Tooru Fujisawa [:arai]]

Attachment: Part 7: Implement native ArraySpeciesCreate wrapper.

native ArraySpeciesCreate is just a wrapper to call self-hosted ArraySpeciesCreate.

Comment 40

[Jan de Mooij [:jandem]]

Comment on attachment 8728862 [details] [diff] [review]
Part 5: Inline IsWrappedArrayConstructor intrinsic.

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

Nice

::: js/src/jit/MCallOptimize.cpp
@@ +2543,5 @@
>      return InliningStatus_Inlined;
>  }
>  
> +static inline bool
> +IsProxyClass(const Class* clasp)

We don't need this as there already is an IsProxyClass in vm/ProxyObject.h (it's used in this file so it should be available).

@@ +2566,5 @@
> +    TemporaryTypeSet* types = arg->resultTypeSet();
> +    switch (types->forAllClasses(constraints(), IsProxyClass)) {
> +      case TemporaryTypeSet::ForAllResult::ALL_FALSE: {
> +        break;
> +      }

Nit: can remove the {} here.

Comment 41

[Eric Faust [:efaust]]

Comment on attachment 8728854 [details] [diff] [review]
Part 1: Implement Array[@@species] getter.

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

This is r+ either way, but I'm curious if we couldn't have one self-hosted function which is cloned into content compartments as the various @@species getters. Most of them are just "return this", right? I don't think it matters much, but it might be a nice simplification.

Comment 42

[Eric Faust [:efaust]]

Comment on attachment 8728856 [details] [diff] [review]
Part 2: Add IsArraySpecies.

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

Looks good.

::: js/src/jsarray.cpp
@@ +872,5 @@
> +{
> +    // This must only return true if v is *the* Array constructor for the
> +    // current compartment; we rely on the fact that any other Array
> +    // constructor would be represented as a wrapper.
> +    return v.isObject() &&

OK, this isn't your fault, since you didn't write the patch, but I find it more intuitive to write this function:

static bool
IsArrayConstructor(const Value& v, JSContext* cx)
{
    return v.isObject() && v.toObject() == cx->global()->getConstructor(JSProto_Array).toObject();
}

What do you think?

Comment 43

[Eric Faust [:efaust]]

Comment on attachment 8728858 [details] [diff] [review]
Part 3: Add IsWrappedArrayConstructor intrinsic.

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

wfm

Comment 44

[Eric Faust [:efaust]]

Comment on attachment 8728880 [details] [diff] [review]
Part 6: Implement self-hosted ArraySpeciesCreate.

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

Ignore my ramblings about rewriting IsArrayConstructor from the earlier part. I see why it's how it is, now.

::: js/src/builtin/Array.js
@@ +889,5 @@
> +    // Step 4, 6.
> +    if (!IsArray(originalArray))
> +        return std_Array(length);
> +
> +    // Steps 3, 5.a.

I think calling this step 3 is just confusing the point. We can call this just 5.a

@@ +893,5 @@
> +    // Steps 3, 5.a.
> +    var C = originalArray.constructor;
> +
> +    // Step 5.b.
> +    if (IsConstructor(C) && IsWrappedArrayConstructor(C))

Now that I think about it, it's nice that we don't have to have a global involved in the IsArrayConstructor check, because it prevents any compartment concerns.

I guess we could do the same thing, except not take the cx, and do v.toObject().compartment()->global()->getConstructor(...) or something. Probably should just leave it ;)

@@ +902,5 @@
> +        // Step 6.c.i.
> +        C = C[std_species];
> +
> +        // Optimized path for an ordinary Array.
> +        if (C === GetBuiltinConstructor("Array"))

IsArrayConstructor(C) also works, right?

@@ +905,5 @@
> +        // Optimized path for an ordinary Array.
> +        if (C === GetBuiltinConstructor("Array"))
> +            return std_Array(length);
> +
> +        // Step 6.c.ii.

Also need the other step with the C is undefined check here. In the latest version that's step 6, but It's probably step 7 in the draft this is commented from.

Comment 45

[Eric Faust [:efaust]]

Comment on attachment 8728882 [details] [diff] [review]
Part 7: Implement native ArraySpeciesCreate wrapper.

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

Thanks for unifying the two implementations.

::: js/src/jsarray.cpp
@@ +931,5 @@
> +    RootedId createId(cx, NameToId(cx->names().ArraySpeciesCreate));
> +    RootedFunction create(cx, JS::GetSelfHostedFunction(cx, "ArraySpeciesCreate", createId, 2));
> +
> +    InvokeArgs args(cx);
> +    if (!args.init(2))

Please use the (brand new) FixedInvokeArgs<2>

@@ +940,5 @@
> +    args[0].setObject(*origArray);
> +    if (length <= INT32_MAX)
> +        args[1].setInt32(length);
> +    else
> +        args[1].setNumber((double)length);

There's a NumberValue(uint32_t) overload that does exactly this. Please use it here.

Comment 46

[Eric Faust [:efaust]]

Comment on attachment 8728881 [details] [diff] [review]
Part 8: Use ArraySpeciesCreate in Array.prototype.concat.

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

::: js/src/builtin/Array.js
@@ +929,5 @@
>      // Step 1.
>      var O = ToObject(this);
>  
>      // Step 2.
> +    var A = ArraySpeciesCreate(O, 0);

blech. I hate that O and 0 look so similar. No change needed, just grumbling. If you wanted to make it |obj|, I wouldn't object, but I understand why the spec names were used.

Comment 47

[Tooru Fujisawa [:arai]]

Thank you for reviewing :D

(In reply to Eric Faust [:efaust] from comment #44)
> Comment on attachment 8728880 [details] [diff] [review]
> Part 6: Implement self-hosted ArraySpeciesCreate.
> @@ +902,5 @@
> > +        // Step 6.c.i.
> > +        C = C[std_species];
> > +
> > +        // Optimized path for an ordinary Array.
> > +        if (C === GetBuiltinConstructor("Array"))
> 
> IsArrayConstructor(C) also works, right?

yeah, but comparison with GetBuiltinConstructor can be done in JIT code, without adding self-hosting instrinsic and inlining for IsArrayConstructor.


> @@ +905,5 @@
> > +        // Optimized path for an ordinary Array.
> > +        if (C === GetBuiltinConstructor("Array"))
> > +            return std_Array(length);
> > +
> > +        // Step 6.c.ii.
> 
> Also need the other step with the C is undefined check here. In the latest
> version that's step 6, but It's probably step 7 in the draft this is
> commented from.

Sorry, those are Steps 5.xxx, and Step 6 (If C is undefined, return ? ArrayCreate(length)) is just below the block

>     // Step 6.
>     if (C === undefined)
>         return std_Array(length);

Comment 48

[Eric Faust [:efaust]]

Comment on attachment 8696166 [details] [diff] [review]
Part 9: Use ArraySpeciesCreate in Array.prototype.map.

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

Thanks for also updating comments!

Comment 49

[Eric Faust [:efaust]]

Comment on attachment 8696167 [details] [diff] [review]
Part 10: Use ArraySpeciesCreate in Array.prototype.slice.

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

wfm

Comment 50

[Eric Faust [:efaust]]

Comment on attachment 8696168 [details] [diff] [review]
Part 11: Use ArraySpeciesCreate in Array.prototype.splice.

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

Strong. Nice refactor.

Comment 51

[Eric Faust [:efaust]]

Comment on attachment 8696169 [details] [diff] [review]
Part 12: Add tests for ArraySpeciesCreate.

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

This is remarkably thorough. Thank you for your attention to detail.

::: js/src/tests/ecma_6/Array/species.js
@@ +45,5 @@
> +
> +      return undefined;
> +    }
> +  });
> +  a = new Proxy([1, 2, 3, 4, 5], {

Thanks for testing the proxy case!

@@ +77,5 @@
> +  a.constructor = {
> +    [Symbol.species]: null
> +  };
> +  b = a[funcName](...args);
> +  assertEq(b.constructor, Array);

Nice.

@@ +115,5 @@
> +  });
> +  b = a[funcName](...args);
> +  assertEq(b.constructor, Array);
> +
> +  // @@species from different global

:)

@@ +149,5 @@
> +  b = g.a[funcName](...args);
> +  assertEq(b.constructor, g.Array);
> +
> +  // subclasses
> +  if (classesEnabled()) {

don't need classesEnabled() anymore :)

Comment 52

[Tooru Fujisawa [:arai]]

Attachment: Part 12.1: Update basic/splice-check-steps.js to follow @@species.

maybe related to bug 892903, basic/splice-check-steps.js now logs "get-constructor," for ArraySpeciesCreate.
just added that to expected_order for each test, that invokes ArraySpeciesCreate.

Comment 53

[Tooru Fujisawa [:arai]]

Comment on attachment 8734609 [details] [diff] [review]
Part 12.1: Update basic/splice-check-steps.js to follow @@species.

r=evilpie on IRC

Comment 54

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8825d25eab2d107c5be515b15ef4b5ee1de79421
Bug 1165052 - Part 1: Implement Array[@@species] getter. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a3147dc1003ff95a0b4b79d924113c1466baec9
Bug 1165052 - Part 2: Add IsArraySpecies. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e02cbf51e4540dec3716e5ec9b415b632e8473c
Bug 1165052 - Part 3: Add IsWrappedArrayConstructor intrinsic. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbb1c215d783a85c27d71ed8bb832690962c948
Bug 1165052 - Part 4: Inline IsConstructor intrinsic. r=jandem

https://hg.mozilla.org/integration/mozilla-inbound/rev/cb171b15a882723b189e2fefa2a06f27e8f907c5
Bug 1165052 - Part 5: Inline IsWrappedArrayConstructor intrinsic. r=jandem

https://hg.mozilla.org/integration/mozilla-inbound/rev/0344adae92f3c3fe026a7c365689343cde287fe1
Bug 1165052 - Part 6: Implement self-hosted ArraySpeciesCreate. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa59399ae8e09a153b63210b966c6364376c1e0c
Bug 1165052 - Part 7: Implement native ArraySpeciesCreate wrapper. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c725d3463f112ca08e0c0a05537a6077a915723
Bug 1165052 - Part 8: Use ArraySpeciesCreate in Array.prototype.concat. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/e49d0dfcaa5c7415c94b830e11dda40700522011
Bug 1165052 - Part 9: Use ArraySpeciesCreate in Array.prototype.filter. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d13efb13fec2a7843d65c27c9862bd5e51c457d
Bug 1165052 - Part 10: Use ArraySpeciesCreate in Array.prototype.map. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/e682faee5bff7c99ed7dc045f67e5324525d396e
Bug 1165052 - Part 11: Use ArraySpeciesCreate in Array.prototype.slice. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e108d752209e5dc17c91775974d0b82b7ff2f93
Bug 1165052 - Part 12: Use ArraySpeciesCreate in Array.prototype.splice. r=efaust,evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/52a142a57bf629018a0b16e85d75450e1699c0da
Bug 1165052 - Part 13: Add tests for ArraySpeciesCreate. r=efaust

Comment 55

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a74723c089e962e2f3f6c5aee205b55b8f17e00
Backed out changeset 52a142a57bf6 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/696987b80ad49e0e1225f6ffcb1e59cb991f4f51
Backed out changeset 5e108d752209 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/b38df48d4638a80a73e3d4dc6d935f1bef122ff7
Backed out changeset e682faee5bff (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/d61e32f1ad9cf2c377317066bb668f7e7bb86e2e
Backed out changeset 5d13efb13fec (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e3a42336a3738b82d35e5fb09d9aef347106ba9
Backed out changeset e49d0dfcaa5c (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/64788d3e93a9b95a3111faf76af4a0f7b690d1d6
Backed out changeset 0c725d3463f1 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/eab50485823b191b712479eb887c00ed21a0e5a5
Backed out changeset aa59399ae8e0 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/70de0d01ec44e7f40ff784bf3aab9fdc5e49cfd8
Backed out changeset 0344adae92f3 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/24ce44d6133013f942fa9c1bf5b641d468da0efc
Backed out changeset cb171b15a882 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/23e771ef64a10cb55b7d938b9d5dd60444c15013
Backed out changeset fdbb1c215d78 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/a08059a8e1785f9d18b776a09e98afcc24a7ad13
Backed out changeset 5e02cbf51e45 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd9eee8108af36b5f26dce4c1934476cc3cd38d5
Backed out changeset 2a3147dc1003 (bug 1165052)

https://hg.mozilla.org/integration/mozilla-inbound/rev/705cf9ac47e9ccb246d18b5c8a0b6ce6419fa556
Backed out changeset 8825d25eab2d (bug 1165052)

Comment 56

[Tooru Fujisawa [:arai]]

Attachment: Part 12.1: Disable test_oom_reporting.html because the test is no more valid.

https://dxr.mozilla.org/mozilla-central/source/dom/bindings/test/test_oom_reporting.html

this test expects Array#splice to execute NewFullyAllocatedArrayTryReuseGroup or some fully allocated variants, to report OOM instantly, but now we use std_Array, and it doesn't fully allocate at the object allocation, and it takes too long time until hit actual OOM while defining properties.

Do you know any other way to emulate this test with other function?
or can we just disable this test?

Comment 57

[Tooru Fujisawa [:arai]]

Attachment: Part 1.1: Add Array[@@species] to xray test.

Related to bug 887016, Array just works by adding Symbol.species to the list.
(not like RegExp, it doesn't work by adding Symbol.species, and also not like TypedArray, that doesn't fail even I don't add Symbol.species)

Comment 58

[Tooru Fujisawa [:arai]]

Comment on attachment 8736216 [details] [diff] [review]
Part 12.1: Disable test_oom_reporting.html because the test is no more valid.

Discussed on IRC.
I'll add a new testing function that throws OOM, and use it in test_oom_reporting.html, instead of Array#splice.

Comment 59

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac7bd86f3e6020d1afd3271bb26e04e146472b29
Bug 1165052 - Part 0: Add throwOutOfMemory testing function and use it instead of Array.prototype.splice in test_oom_reporting.html. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/35b9afd414ed452fdfee91b89a6dc51de9e3868c
Bug 1165052 - Part 1: Implement Array[@@species] getter. r=efaust,bholley

https://hg.mozilla.org/integration/mozilla-inbound/rev/f15991a13732a4d966fad452bfc8f769fb258859
Bug 1165052 - Part 2: Add IsArraySpecies. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae46f7e09a221fa5006e4abd2ba458998328d5c4
Bug 1165052 - Part 3: Add IsWrappedArrayConstructor intrinsic. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecb8f8821de2ab318eb4a0d863b5d9c1cd97d70f
Bug 1165052 - Part 4: Inline IsConstructor intrinsic. r=jandem

https://hg.mozilla.org/integration/mozilla-inbound/rev/62974e03b350265b4413963e34ccef88bdb9d3d4
Bug 1165052 - Part 5: Inline IsWrappedArrayConstructor intrinsic. r=jandem

https://hg.mozilla.org/integration/mozilla-inbound/rev/a06b6d9f65402fb736f55ab5b378dd3759adf9f7
Bug 1165052 - Part 6: Implement self-hosted ArraySpeciesCreate. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/450ea0e5c8cb17e38ac80c456b8674bfa4ade377
Bug 1165052 - Part 7: Implement native ArraySpeciesCreate wrapper. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2ae0fa9376081d2b7c509f213458bb850ae27d8
Bug 1165052 - Part 8: Use ArraySpeciesCreate in Array.prototype.concat. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc13616aed735278e640805522ec4866f16305de
Bug 1165052 - Part 9: Use ArraySpeciesCreate in Array.prototype.filter. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7d9523b227c6c30f8a894bacdeeefb6b05d25aa
Bug 1165052 - Part 10: Use ArraySpeciesCreate in Array.prototype.map. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/830dc8e6e370b2a41b2a175294334ca60543c8bd
Bug 1165052 - Part 11: Use ArraySpeciesCreate in Array.prototype.slice. r=efaust

https://hg.mozilla.org/integration/mozilla-inbound/rev/85fc8ee6d839e85c5f7a5880b94294befce00be8
Bug 1165052 - Part 12: Use ArraySpeciesCreate in Array.prototype.splice. r=efaust,evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/28226109634e0589f8ce078ee4985033bc64b08a
Bug 1165052 - Part 13: Add tests for ArraySpeciesCreate. r=efaust

Comment 60

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ac7bd86f3e60
https://hg.mozilla.org/mozilla-central/rev/35b9afd414ed
https://hg.mozilla.org/mozilla-central/rev/f15991a13732
https://hg.mozilla.org/mozilla-central/rev/ae46f7e09a22
https://hg.mozilla.org/mozilla-central/rev/ecb8f8821de2
https://hg.mozilla.org/mozilla-central/rev/62974e03b350
https://hg.mozilla.org/mozilla-central/rev/a06b6d9f6540
https://hg.mozilla.org/mozilla-central/rev/450ea0e5c8cb
https://hg.mozilla.org/mozilla-central/rev/b2ae0fa93760
https://hg.mozilla.org/mozilla-central/rev/fc13616aed73
https://hg.mozilla.org/mozilla-central/rev/f7d9523b227c
https://hg.mozilla.org/mozilla-central/rev/830dc8e6e370
https://hg.mozilla.org/mozilla-central/rev/85fc8ee6d839
https://hg.mozilla.org/mozilla-central/rev/28226109634e

Comment 61

[Tooru Fujisawa [:arai]]

Updated following documents

https://developer.mozilla.org/en-US/Firefox/Releases/48
https://developer.mozilla.org/en-US/docs/Web/JavaScript/New_in_JavaScript/ECMAScript_6_support_in_Mozilla

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/@@species

Comment 62

[Florian Scholz (Open Web Docs)]

Great, thank you for the doc updates!