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

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 18:04:18 GMT, Claes Redestad wrote: >> 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

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

2024-02-20 Thread Claes Redestad
On Tue, 20 Feb 2024 18:08:11 GMT, Aleksey Shipilev wrote: >> I couldn't figure out why we'd want to have `String::substring` micros in a >> test `StringBuffers` (there's also a `StringSubstring` micro), though I >> could deal with that as an explicit cleanup RFE. > > Yeah, I would like to keep

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

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 18:00:42 GMT, Claes Redestad wrote: >> test/micro/org/openjdk/bench/java/lang/StringBuffers.java line 49: >> >>> 47: >>> 48: @Benchmark >>> 49: public String emptyToString() { >> >> Do we actually need to remove the `substring` test here? > > I couldn't figure out w

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

2024-02-20 Thread Claes Redestad
On Tue, 20 Feb 2024 17:02:40 GMT, Aleksey Shipilev 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/ja

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

2024-02-20 Thread Claes Redestad
> JDK-8282429 accidentally removed an optimization (JDK-8240094) that ensured > StringBuilder/StringBuffer::toString returns `""` when the builders are empty. > > > Name Cnt Base Error Test Error Unit > Change > StringBuffers.emptyToString5 12,289 ±