On Mon, 10 Apr 2023 13:17:39 GMT, jmehrens <d...@openjdk.org> wrote:
>> 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.
That if/else looks like a job for Math.clamp. Something like:
var si = element.spliterator();
String[] elems = new String[Math.clamp(si.estimateSize(), 8,
ArraysSupport.SOFT_MAX_ARRAY_LENGTH)];
Perhaps we need to pad the estimate a little bit to try and avoid one resize:
var si = element.spliterator();
String[] elems = new String[Math.clamp(si.estimateSize() + 8L, 8,
ArraysSupport.SOFT_MAX_ARRAY_LENGTH)];
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13383#discussion_r1161902142
