RFR: 8301958: Avoid Arrays.copyOfRange overhead in java.lang.String [v5]

[Stuart Marks]

On Tue, 7 Feb 2023 15:25:05 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> This adds a local, specialized `copyBytes` method to `String` that avoids certain redundant range checks and clamping that JIT has issues removing fully.
>> 
>> This has a small but statistically significant effect on `String` microbenchmarks, eg.:
>> 
>> Baseline
>> 
>> Benchmark                                            (size)  Mode  Cnt   Score   Error  Units
>> StringConstructor.newStringFromArray                      7  avgt   15  16.817 ± 0.369  ns/op
>> StringConstructor.newStringFromArrayWithCharset           7  avgt   15  16.866 ± 0.449  ns/op
>> StringConstructor.newStringFromArrayWithCharsetName       7  avgt   15  22.198 ± 0.396  ns/op
>> 
>> 
>> Patch:
>> 
>> Benchmark                                            (size)  Mode  Cnt   Score   Error  Units
>> StringConstructor.newStringFromArray                      7  avgt   15  15.477 ± 0.342  ns/op
>> StringConstructor.newStringFromArrayWithCharset           7  avgt   15  15.557 ± 0.352  ns/op
>> StringConstructor.newStringFromArrayWithCharsetName       7  avgt   15  21.272 ± 0.398  ns/op
>> 
>> 
>> Care has to be taken to ensure preconditions have been checked when using `checkBytes`. In the case of `String(AbstractStringBuilder)` there's a possible pre-existing issue where the constructor might either throw an exception or truncate the buffer if the builder byte array and length is not in agreement (theoretically possible if you clear/remove and call `trimToSize()` concurrently). Adding an explicit check here seem to be the right thing to do regardless of this RFE.
>
> Claes Redestad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   copyrights

Marked as reviewed by smarks (Reviewer).

src/java.base/share/classes/java/lang/StringLatin1.java line 763:

> 761:             return "";
> 762:         }
> 763:         return new String(String.copyBytes(val, index, len), LATIN1);

It might be worthwhile putting a comment at the top of this method saying that the caller is required to bounds-check the arguments. It's called from several other classes in this package, so it would be good to document this package-internal contract. I checked the callers and they seem fine.

-------------

PR: https://git.openjdk.org/jdk/pull/12453