RFR: 8180352: Add Stream.toList() method [v3]

[Stuart Marks]

On Mon, 23 Nov 2020 14:06:02 GMT, sergus13 <github.com+74766043+sergus13 at openjdk.org> wrote:

>> This implementation duplicates the array twice, once in this.toArray() and once in the constructor of ArrayList that calls toArray() on Collections.ArrayList.
>> 
>> I'm sure there is a better way
>
> In `ReferencePipeline` class we have:
> 
>     @Override
>     public final Object[] toArray() {
>         return toArray(Object[]::new);
>     }
> 
>     @Override
>     @SuppressWarnings("unchecked")
>     public final <A> A[] toArray(IntFunction<A[]> generator) {
>         ...
>         @SuppressWarnings("rawtypes")
>         IntFunction rawGenerator = (IntFunction) generator;
>         return (A[]) Nodes.flatten(evaluateToArrayNode(rawGenerator), rawGenerator)
>                               .asArray(rawGenerator);
>     }
> 
> 
> In `Nodes` class we have:
> 
> 
>     public static <T> Node<T> flatten(Node<T> node, IntFunction<T[]> generator) {
>         if (node.getChildCount() > 0) {
>             ... 
>             T[] array = generator.apply((int) size);
>             new ToArrayTask.OfRef<>(node, array, 0).invoke();
>             return node(array);
>         } else {
>             return node;
>         }
>     }
> 
> 
> 
> It looks like it is required to implement `toList` method in a similar way in order to avoid array copy. i.e. there will be `IntFunction<List<T>> generator` which will generate 'ArrayList' with specified size and the list's `add` method will be called to add elements to the list.

This is the default implementation in the Stream interface, which is overridden by an implementation in the ReferencePipeline class, so it will rarely be used in normal operation. The ReferencePipeline version (part of this changeset) is based on toArray() and avoids any copying. I'm thus not inclined to add new interfaces in order to support this default implementation.

As written it's true that the default implementation does perform apparently redundant copies, but we can't be assured that toArray() actually returns a freshly created array. Thus, we wrap it using Arrays.asList and then copy it using the ArrayList constructor. This is unfortunate but necessary to avoid situations where someone could hold a reference to the internal array of a List, allowing modification of a List that's supposed to be unmodifiable.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1026