On Wed, 25 Nov 2020 09:52:28 GMT, Claes Redestad <redes...@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
