On Sun, 14 Apr 2024 14:33:26 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> What are the scenarios which had regressions? >> Given the conservative growth for StringBuilder, it surprises me a bit that >> any scenario would regress. > > I took a second look and it turns out that there were neither regressions nor > improvements in my test, only a few false positives: For C2 the SB chain is > recognized by the (fragile) C2 OptimizeStringConcat pass and transformed into > a shape where the initial size in java bytecode - if any - is ignored. > > With C1 having an initial size can have a significant effect. One way to > provoke a regression there is to have a huge constant suffix while > underestimating the size of the operands, which can lead to excessive > allocation: > > > Name Cnt Base Error Test Error > Unit Change > StringConcat.concat23StringConst 5 385,268 ± 5,238 341,251 ± 2,606 > ns/op 1,13x (p = 0,000*) > :gc.alloc.rate 6039,095 ± 82,309 12764,169 ± 97,146 > MB/sec 2,11x (p = 0,000*) > :gc.alloc.rate.norm 2440,003 ± 0,000 4568,002 ± 0,000 > B/op 1,87x (p = 0,000*) > > > Still a bit better on throughput from less actual copying. > > *If* the `StringBuilder` is sized sufficiently (constants + args * N) things > look much better, of course: > > -XX:TieredStopAtLevel=1 > Name Cnt Base Error Test Error > Unit Change > StringConcat.concat23StringConst 5 385,268 ± 5,238 259,628 ± 2,515 > ns/op 1,48x (p = 0,000*) > :gc.alloc.rate 6039,095 ± 82,309 8902,803 ± 86,563 > MB/sec 1,47x (p = 0,000*) > :gc.alloc.rate.norm 2440,003 ± 0,000 2424,002 ± 0,000 > B/op 0,99x (p = 0,000*) > > > For most cases having a size based on the sum of the constants plus some > small factor per argument seem to be a decent heuristic - for C1. Since this > adds just one bytecode to the generated method I guess it wouldn't hurt. Presizing this StringBuilder is a thing I looked into once upon a time, discussed here: https://mail.openjdk.org/pipermail/compiler-dev/2015-March/009356.html This work, I understand, the indyfication of string concatenation in the first place. Primitive values can get bounded at appropriate lengths for their types (see e.g. https://stackoverflow.com/a/21146952/869736). It sounds like you're trying to conserve bytecode length, so maybe you don't want to presize the StringBuilder with the exact Object.toString() lengths, though. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18690#discussion_r1576819289
