On Mon, 10 Apr 2023 17:05:28 GMT, jmehrens <d...@openjdk.org> wrote:
>> 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)];
Note that this also has to handle the case where `si.estimateSize()` returns
values close to `Long.MAX_VALUE`:
final var si = elements.spliterator();
final long est = si.estimateSize();
final int length;
final long MIN_LENGTH = 8;
if ((si.hasCharacteristics() & Spliterator.SIZED) != 0) {
length = (int) Math.clamp(est, MIN_LENGTH,
ArraysSupport.SOFT_MAX_ARRAY_LENGTH);
} else if (est == Long.MAX_VALUE) {
length = MIN_LENGTH;
} else if (est >= ArraysSupport.SOFT_MAX_ARRAY_LENGTH - MIN_LENGTH) {
length = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;
} else {
length = (int) Math.clamp(est + MIN_LENGTH, MIN_LENGTH,
ArraysSupport.SOFT_MAX_ARRAY_LENGTH);
}
var elems = new String[length];
int size = 0;
for (final var it = Spliterators.iterator(si); it.hasNext();) {
CharSequence cs = it.next();
// rest of method...
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13383#discussion_r1161920643
