On Tue, 20 Feb 2024 17:02:40 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> Claes Redestad has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Revert StringBuffers removals
>> - Update from review comments by @shipilev
>
> src/java.base/share/classes/java/lang/StringBuilder.java line 478:
>
>> 476: }
>> 477: // Create a copy, don't share the array
>> 478: return new String(this, null);
>
> Ok, this got me a bit confused, but I think this just inlines the call to
> this constructor:
>
>
> public String(StringBuilder builder) {
> this(builder, null);
> }
Yes, I was mostly reaching for increased consistency with `StringBuffer` here.
> test/micro/org/openjdk/bench/java/lang/StringBuilderToString.java line 33:
>
>> 31: import org.openjdk.jmh.annotations.Param;
>> 32: import org.openjdk.jmh.annotations.Scope;
>> 33: import org.openjdk.jmh.annotations.Setup;
>
> Is this needed?
It's needed again now that I reverted the code removals.. :-)
> test/micro/org/openjdk/bench/java/lang/StringBuilders.java line 407:
>
>> 405:
>> 406: private void generateData() {
>> 407: sb = new StringBuilder(length);
>
> This looks weird, given there is a `sb` initialization two lines below. Is
> this `sbEmpty`, really?
Yes, updating the name to avoid the confusing shadowing.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496273112
PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496282094
PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496275704
