archives

« Bugzilla Issues Index

#2416 — 22.1.2.1 usingIterator=true when Array.from's arg is an Array


Since 22.1.3.30 specifies that Array.prototype.@@iterator exists, the test in step 6 of Array.from (22.1.2.1) will set usingIterator to true when the "arrayLike" is an actual Array. This doesn't seem correct, it means that the length information in the passed array will be ignored, the result can't be preallocated to the correct length, and sparse elements skipped:

var a = [];
a[1] = 'a';
a[9] = 'b';
Array.from(a) // ['a','b']?

I suggest that step 6a ought to read something like, "If Array.isArray(items) is true, the set usingIterator to false." (or "If items is an exotic Array object..."). Since typed arrays also have iterators, you might also want to include a step 6b, "If items is a typed array...", so that the following works as expected:

var a = new Uint8Array(2);
var b = Array.from.call(Uint8Array, a); // 0-length array?

As the algorithm is currently written, the Uint8Array constructor would be called with no arguments and the result of Array.from() would be a zero-length array.

If the behavior in the spec as it is currently written is desired, the author could pass the iterator explicitly:

Array.from(a.values()) // collapses sparse array
Array.from.call(Uint8Array, a.values()) // makes zero-length Uint8Array

So no functionality is lost by making Array and TypedArray *not* use the usingIterator path.

The algorithm in 22.2.2.1 (%TypedArray%.from) should also be similarly updated to match the fixes to Array.from.


It is not possible to apply Array.from() on TypedArray constructors, regardless of whether the @@iterator or .length path is taken. Either CreateDataPropertyOrThrow() or the final Put() will throw a TypeError when `A` is TypedArray object.

And array iterators don't skip holes, that means `0 in Array.from([,])` yields true.

Related:
http://esdiscuss.org/topic/overly-complicated-array-from
http://esdiscuss.org/topic/why-does-array-from-accept-non-iterable-arraylikes


Sorry about the sparse thing, I was looking at es6-shim's implementation, which had a hole-skipping ArrayIterator that must be left over from some earlier version of the spec. I'll submit a patch to es6-shim.

I think the TypedArray case is still significant.

u = Uint8Array(8)
u["1"] = 2
u.length = 8

doesn't currently throw an exception, why would Array.from() do so? It seems like Array.from should work with TypedArray if it is to be properly generic. (Or a @TypedArray@.from() method could be added, but -- ugh.)

And it seems like there are still significant performance gains to be had by preallocating arrays. That holds even for custom classes:

Array.from.call(MyClass, [1, 2, 3])

The MyClass constructor might be also able to benefit from knowing the length of the array up-front.


(In reply to comment #2)
> I think the TypedArray case is still significant.
>
> u = Uint8Array(8)
> u["1"] = 2
> u.length = 8
>
> doesn't currently throw an exception, why would Array.from() do so?

It does not throw an exception, because the JavaScript implementation you're using does not comply to the current ES6 draft. ;-)

V8 bleeding edge (rev18548, version 3.24.15):
---
d8> "use strict"; var u = new Uint8Array(8); u.length = 8
(d8):1: TypeError: Cannot set property length of #<Uint8Array> which has only a getter
"use strict"; var u = new Uint8Array(8); u.length = 8
^
TypeError: Cannot set property length of #<Uint8Array> which has only a getter
at (d8):1:51
---

The "use strict" directive is important to get the same semantics as in `Put(A, "length", len, true)`, the last parameter defines the strictness of the Put() operation.


> And it seems like there are still significant performance gains to be had by
> preallocating arrays. That holds even for custom classes:
>
> Array.from.call(MyClass, [1, 2, 3])
>
> The MyClass constructor might be also able to benefit from knowing the length
> of the array up-front.

For the standard Array it's actually not too import which path is preferred, as long as there are no funny properties (read: accessor properties) which introduce arbitrary side effects. Changes like https://bugzil.la/952891 will further improve array iteration. And custom classes can use the TypedArray.from() function if it's necessary or helpful to know the expected length in the constructor invocation.

(Note: I'm not strictly in favour of one or the other approach, I'm merely providing some input for this discussion.)


I'd like to go back to sparse array's case, as I think it's still not right.

Indeed result array's length will match length of input array, but it won't be perfect copy of it.

var input = [1,,2];
var result = Array.from(input); // [1,undefined,2];

input.hasOwnProperty(1); // false
result.hasOwnProperty(1); // true
input.forEach(x => console.log(x)); // 1, 2
result.forEach(x => console.log(x)); // 1, undefined, 2

It seems not consistent, especially that there are cases for which Array.from will produce sparse arrays:

Array.from({0: 1, 2: 2, length: 2 }); // 1,,2 (sparse array)

I thik as C. Scott Ananian propopsed, arrays should go through logic specified for `usingIterator = true`.

If it stays as it is it makes this function not viable to produce array copies, which is very common use case, currently addressed with `arr.slice()`, and as `arr.slice()` will now create objects of input type it's no longer safe for creation of plain arrays, it'll be great if Array.from could be used for that.


Sorry, of course I meant:

====
(...)
Array.from({0: 1, 2: 2, length: 3 }); // 1,,2 (sparse array)

I think as C. Scott Ananian propopsed, arrays should *not* go through logic specified for `usingIterator = true`.
(...)
====


Array.from was discussed at the last TC39 meeting. See https://github.com/rwaldron/tc39-notes/blob/master/es6/2014-01/jan-28.md#arrayfrom

I don't think those notes capture the entire discussion, but the conclusion is clear: "Keep as spec'ed."

However, I don't think the points brought up here were fully explored at the meeting.

In particular, I don't think it was clear to everybody that:

Array.from([1,,3,,5])

returns [1,3,5] rather than [1,undefined,3,undefined,5].

I think an appropriate next step would be for Scott to start a new es-discuss thread that specifically addresses that concern and references this bug.

Allen


(In reply to comment #6)

>
> In particular, I don't think it was clear to everybody that:
>
> Array.from([1,,3,,5])
>
> returns [1,3,5] rather than [1,undefined,3,undefined,5].
>

I'm not sure where my head was sutick when I wrote the above, but it's wrong.

As currently specified,
Array.from([1,,3,,5])
returns [1,undefined,3,undefined,5]

Array Array.from uses @@interator which for Array.prototype is the same as Array.prototype.values. the values iterator for arrays returns undefined as the value of sparse holes.


Let's open a new bug for whether Array.from({0: 1, 2: 2, length: 3 }) should result in a sparse array.

This bug is *not* about behavior changes, just whether ArrayIterator should be involved in Array.from([ ... ]). This is purely an efficiency issue: since the invocation of Array.@@iterator is potentially observable, are we foreclosing optimization opportunities by forcing Array.from() to go through the iterator path.

Input from implementors is probably called for.


Bug 2562 is for the "sparse arrays from array-like" issue.


You are essentially talking about enabling an implementation to inline the IsIterable (now called CheckIterable) check. This check is potentially observable via a proxy or if @@iterator is an accessor property.

However, this type of observability is possible for pretty much every potential inline site so an implementation that is going to do inlining is going to have to have a mechanism for determining whether or not any specific inlining can be done unobservably.

So, this seems like a general issue that optimizing implementation have to take into account and something that we shouldn't try to special case just for this one specific instance of the problem.