----- Mail original -----
> De: "Stuart Marks" <stuart.ma...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Vendredi 15 Juin 2018 03:02:04
> Objet: RFR(s): 8060192: Add default method Collection.toArray(generator)
> Hi all,
Hi Stuart,
>
> 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.
for ArrayList, you may use a code like this,
<T> T[] toArray(IntFunction<T[]> generator) {
return (T[]) Arrays.copyOf(elementData, size,
generator.apply(0).getClass());
}
so you win only one comparison (yeah !), which can be perfectly predicted, so
you should not see any perf difference :)
>
> # 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.
List<String>.class or List<String>[].class do not work either.
>
> For these reasons I'd like to proceed with adding toArray(generator) API.
>
so thumb up for me !
> Thanks,
>
> s'marks
Rémi
>
>
> [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
