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

jmehrens duke at openjdk.org
Mon Apr 10 17:08:48 UTC 2023


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


More information about the core-libs-dev mailing list