On Tue, 7 Feb 2023 15:25:05 GMT, Claes Redestad <[email protected]> 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
Thanks @cl4es to look into this!
src/java.base/share/classes/java/lang/String.java line 698:
> 696: }
> 697:
> 698: static byte[] copyBytes(byte[] bytes, int offset, int length) {
Given that the stub generated for array copy seems highly dependent by the call
site constrains, did you tried adding a check for offset == 0 and/or length ==
bytes.length?
If (offset == 0 && bytes.length == length) {
System.arrayCopy(bytes, 0, dst, bytes.length);
// etc etc the other combinations
This should have different generated stubs with much smaller ASM depending by
the enforced constrains
-------------
Changes requested by [email protected] (no known OpenJDK username).
PR: https://git.openjdk.org/jdk/pull/12453
