On Tue, 20 Feb 2024 16:32:54 GMT, Claes Redestad <redes...@openjdk.org> wrote:
> 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.emptyToString 5 12,289 ± 0,384 9,883 ± 0,721 ns/op > 1,24x (p = 0,000*) > :gc.alloc.rate 1862,398 ± 57,647 0,007 ± 0,000 MB/sec > 0,00x (p = 0,000*) > :gc.alloc.rate.norm 24,000 ± 0,000 0,000 ± 0,000 B/op > 0,00x (p = 0,000*) > :gc.count 31,000 0,000 counts > :gc.time 21,000 ms > StringBuilders.emptyToString 5 4,146 ± 0,567 0,646 ± 0,003 ns/op > 6,42x (p = 0,000*) > :gc.alloc.rate 9208,656 ± 1234,399 0,007 ± 0,000 MB/sec > 0,00x (p = 0,000*) > :gc.alloc.rate.norm 40,000 ± 0,000 0,000 ± 0,000 B/op > 0,00x (p = 0,000*) > :gc.count 96,000 0,000 counts > :gc.time 64,000 ms > * = significant OK, so before [JDK-8282429](https://bugs.openjdk.org/browse/JDK-8282429) this was handled by `String{Latin1|UTF16}.newString`, gotcha. 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); } 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? 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? 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? ------------- PR Review: https://git.openjdk.org/jdk/pull/17931#pullrequestreview-1890982087 PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496181978 PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496178639 PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496177666 PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496182725
