RFR: 8254082: AbstractStringBuilder.insert(int dstOffset, CharSequence s, int start, int end) is missing fast-path for String [v2]

[Сергей Цыпанов]

On Wed, 25 Nov 2020 09:52:28 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Сергей Цыпанов has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into asb
>>  - 8254082: Add fast-path for String into AbstractStringBuilder.insert(int, CharSequence, int, int)
>
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1716:
> 
>> 1714:     }
>> 1715: 
>> 1716:     private void putCharsAt(int index, String s, int off, int end) {
> 
> Comparing this with `putStringAt(int index, String str)` below I think begs for some consolidation here. I think we either should add a `getBytes(value, index, coder, length)`, or - perhaps preferably? - factor out the package private `String.getBytes` and implement it here in ASB using `s.value()`

I've renamed `putCharsAt` -> `putStringAt`.

As of factoring out `String.getBytes` I believe we cannot do it, as it's referenced also from `StringConcatHelper`. Instead I suggest jusy to declare an overloaded method `getBytes(value, index, coder, length)`.

> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1721:
> 
>> 1719:             return;
>> 1720:         }
>> 1721:         inflate();
> 
> Like in `String.getBytes(byte[], int, byte)` I think we could do an `arraycopy` if both are `false` too, just need to carefully adjust the `index` et.c. In fact the only case that can't use an `arraycopy` in the end is when `s.isLatin1()` and the current sb is already inflated (that's what the `StringLatin1.inflate` branch does in `getBytes`). 
> 
> I think if you consolidate/merge this with the logic in `String.getBytes(byte[], int, byte)` as suggested you'll end up with a sizeable improvement on non-latin1 cases too.

Done

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

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