RFR(s): 8060192: Add default method Collection.toArray(generator)

[Stuart Marks]

Hi all,

I wanted to restart review of the changeset for bug JDK-8060192 [1]. The latest 
webrev is here: [2].

The previous review was from December 2017: [3]. The only difference between the 
current changeset and the previous one is the removal of the overriding 
implementations (see explanation below). Much of the discussion in the previous 
review thread was around performance and the shape of the API.

# Performance

I previously had done some benchmarking on this and reported the results: [4]. 
(I've recently re-done the benchmarks and conclusions are essentially the same.)

The upshot is that implementations that use Arrays.copyOf() are the fastest, 
probably because it's an intrinsic. It can avoid zero-filling the freshly 
allocated array because it knows the entire array contents will be overwritten. 
This is true regardless of what the public API looks like. The default 
implementation calls generator.apply(0) to get a zero-length array and then 
simply calls toArray(T[]). This goes through the Arrays.copyOf() fast path, so 
it's essentially the same speed as toArray(new T[0]).

The overrides I had previously provided in specific implementation classes like 
ArrayList actually are slower, because the allocation of the array is done 
separately from filling it. This necessitates the extra zero-filling step. Thus, 
I've removed the overrides.

# API Shape

There was some discussion about whether it would be preferable to have a Class 
parameter as a type token for the array component type or the array type itself, 
instead of using an IntFunction generator. Most of this boils down to what 
people are comfortable with. However, there are a few points that make the 
generator function approach preferable.

One point in favor of using a generator is that Stream already has a similar 
toArray(generator) method.

Comparing this to the type token alternatives, for simple tasks like converting 
Collection<String> to String[], things are about equal:

     toArray(String[]::new)
     toArray(String.class)
     toArray(String[].class)

However, things are hairier if the element type of the collection is generic, 
such as Collection<List<String>>. The result type is a generic array 
List<String>[]. Using class literals or array constructor references will 
necessitate using an unchecked cast, because none of these can be used to 
represent a generic type.

However, it's possible to use a method reference to provide a properly generic 
IntFunction. This would enable the toArray(generator) method to be used without 
any unchecked casts. (The method it refers to might need have an unchecked cast 
within it, though.) Such a method could also be reused for the corresponding 
Stream.toArray(generator) method.

For these reasons I'd like to proceed with adding toArray(generator) API.

Thanks,

s'marks


[1] https://bugs.openjdk.java.net/browse/JDK-8060192

[2] http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.3/

[3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050325.html

[4] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050585.html