On Tue, 24 Nov 2020 03:46:38 GMT, Stuart Marks <sma...@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
