Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v3]
On Tue, 20 Feb 2024 18:42:16 GMT, Claes Redestad 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.emptyToString5 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.count31,000 0,000 counts >> :gc.time 21,000 ms >> StringBuilders.emptyToString 54,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.count96,000 0,000 counts >> :gc.time 64,000 ms >> * = significant > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Revert accidental import Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/17931#issuecomment-1955020047
Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v3]
On Tue, 20 Feb 2024 18:38:48 GMT, Claes Redestad 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.emptyToString5 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.count31,000 0,000 counts >> :gc.time 21,000 ms >> StringBuilders.emptyToString 54,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.count96,000 0,000 counts >> :gc.time 64,000 ms >> * = significant > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Revert accidental import Marked as reviewed by shade (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17931#pullrequestreview-1891220541
Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v3]
> 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 ±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.count31,000 0,000 counts > :gc.time 21,000 ms > StringBuilders.emptyToString 54,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.count96,000 0,000 counts > :gc.time 64,000 ms > * = significant Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Revert accidental import - Changes: - all: https://git.openjdk.org/jdk/pull/17931/files - new: https://git.openjdk.org/jdk/pull/17931/files/40cca3e3..1846746b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17931&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17931&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17931.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17931/head:pull/17931 PR: https://git.openjdk.org/jdk/pull/17931
Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v3]
On Tue, 20 Feb 2024 18:30:25 GMT, Aleksey Shipilev wrote: >> It's needed again now that I reverted the code removals.. :-) > > Mhm. I don't see any new `@Setup` methods anywhere. Just checked out the PR > locally, removed this import and benchmarks still build, and > `StringBuilderToString` bench still runs. Am I missing something here? Gah, I was looking in the wrong place. Fixed. - PR Review Comment: https://git.openjdk.org/jdk/pull/17931#discussion_r1496321177
