On Mon, 10 Apr 2023 05:46:23 GMT, Tingjun Yuan <[email protected]> wrote:
>> In the current implementation of `String.join(CharSequence, Iterable)`, the
>> temp array `elems` is always initialized with a length of 8. It will cause
>> many array recreations when the `Iterable` contains more than 8 elements.
>> Furthermore, it's very common that an `Iterable` is also a `Collection`. So
>> if the `Iterable` is an instance of `Collection`, the initial length of the
>> array can be `((Collection<?>)elements).size()`. It will not change the
>> current behavior even if the `Collection` is modified asynchronously.
>>
>> I don't know whether this change requires a CSR request.
>
> Tingjun Yuan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> use spliterator().estimateSize()
src/java.base/share/classes/java/lang/String.java line 3466:
> 3464: }
> 3465: int size = 0;
> 3466: for (CharSequence cs: elements) {
I would think you have to locally store the result of `elements.spliterator()`
and then use Spliterators::iterator to adapt it back to an iterator. This
should correctly handle
[early-binding](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Spliterator.html#binding)
spliterators.
I think in the loop the code should use ArraysSupport.newLength.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13383#discussion_r1161712521
