RFR: 8325730: StringBuilder.toString allocation for the empty String [v2]

[Claes Redestad]

On Tue, 20 Feb 2024 17:02:40 GMT, Aleksey Shipilev <shade at 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