On Mon, 23 Nov 2020 14:06:02 GMT, sergus13 
<github.com+74766043+sergu...@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

Reply via email to