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

[Peter Levart]

On Tue, 24 Nov 2020 03:46:38 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> 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.

An alternative with similar performance would be to do a Stream.toArray() and then copy that array into new Object[] and then wrap that copy with listFromTrustedArrayNullsAllowed(). The difference would be in the serialization format of the resulting List and maybe also in the access performance of resulting List (no indirection via the unmodifiableList wrapper and different type for JIT to speculate about). So if we want the resulting List to behave exactly the same in both implementations of toList(), then this alternative might be preferable. WDYT?

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

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