RFR: 8270160: Remove redundant bounds check from AbstractStringBuilder.charAt() [v4]

[Claes Redestad]

On Mon, 26 Jul 2021 08:22:28 GMT, Сергей Цыпанов <github.com+10835776+stsypanov at openjdk.org> wrote:

>> `AbstractStringBuilder.charAt(int)` does bounds check before calling `charAt()` (for non-latin Strings):
>> 
>> @Override
>> public char charAt(int index) {
>>     checkIndex(index, count);
>>     if (isLatin1()) {
>>         return (char)(value[index] & 0xff);
>>     }
>>     return StringUTF16.charAt(value, index);
>> }
>> 
>> This can be improved by removing bounds check from ASB.charAt() in favour of one in String*.charAt(). This gives slight improvement:
>> 
>> before
>> Benchmark                           Mode  Cnt  Score   Error  Units
>> StringBuilderCharAtBenchmark.latin  avgt   50  2,827 ± 0,024  ns/op
>> StringBuilderCharAtBenchmark.utf    avgt   50  2,985 ± 0,020  ns/op
>> 
>> after
>> Benchmark                           Mode  Cnt  Score   Error  Units
>> StringBuilderCharAtBenchmark.latin  avgt   50  2,434 ± 0,004  ns/op
>> StringBuilderCharAtBenchmark.utf    avgt   50  2,631 ± 0,004  ns/op
>
> Сергей Цыпанов has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into 8270160
>  - Merge branch 'master' into 8270160
>    
>    # Conflicts:
>    #	src/java.base/share/classes/java/lang/StringLatin1.java
>  - 8270160: Remove redundant bounds check from AbstractStringBuilder.charAt()

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 227:

> 225:     private void ensureCapacityInternal(int minimumCapacity) {
> 226:         // overflow-conscious code
> 227:         int oldCapacity = capacity();

This doesn't seem related to the intent of the patch, and might distort startup profiles a bit.

src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 352:

> 350:     @Override
> 351:     public char charAt(int index) {
> 352:         checkIndex(index, count);

This is good and harmonizes with the implementation in `String`.

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

> 45:     public static char charAt(byte[] value, int index) {
> 46:         checkIndex(index, value.length);
> 47:         return getChar(value, index);

Seems unrelated to the gist of the patch, and again might skew startup profiles.

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

PR: https://git.openjdk.java.net/jdk/pull/4738