RFR: 8305774: String.join(CharSequence, Iterable) can be optimized if Iterable is a Collection [v3]

[ExE Boss]

On Mon, 10 Apr 2023 17:05:28 GMT, jmehrens <duke at 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