Suggestion : continuous calls of StringBuilder.append

[o0lwj0o]

Hi, I have found some continuous calls about StringBuilder.append(..) in some files. For example, there are several continuous calls from line 164 to 178 in the file https://github.com/SquidPony/SquidLib/blob/master/squidlib-util/src/main/java/squidpony/squidmath/RegionMap.java

while (i-- > 0) { short[] key = keyTable[i]; if (key == null) continue; buffer.append("Packed Region:"); buffer.append(CoordPacker.encodeASCII(key)); buffer.append('='); buffer.append(valueTable[i]); break; } while (i-- > 0) { short[] key = keyTable[i]; if (key == null) continue; buffer.append(separator); buffer.append("Packed Region:"); buffer.append(CoordPacker.encodeASCII(key)); buffer.append('='); buffer.append(valueTable[i]); }

If it was achieved like StringBuilder.append(..).append(..)append(..), it will promote its performance.

[tommyettinger]

I'm a little curious about this one. Is there some documentation on how a chained series of method calls would reliably improve the performance of StringBuilder? HotSpot tends to be rather good at ignoring paths that don't matter, and I don't think either of us have benchmarked this particular toString() implementation. The majority of the time in RegionMap's toString() code is probably spent on garbage-collecting the char array and String allocated in CoordPacker.encodeASCII(), and inlining encodeASCII() so it directly appends to the StringBuilder (without creating temporary objects) could be a bigger gain for performance. However, I don't usually consider toString() implementations as likely bottlenecks unless there are bugs in them (such as deep recursive calls that may overflow the stack); optimizing toString() on an object that probably isn't used thousands of times in a program, and definitely shouldn't be called every time a frame is rendered, will have no effect on the runtime performance of a program. This could affect a database-like application where Strings need to be produced much more quickly than a person can read them, but in games the needs are wholly different, and producing huge amounts of text in real-time is more than anyone could hope to read or use.

I'll leave this open so you can give some background on why StringBuilder.append(..).append(..)append(..) would improve the performance of this method, and if it's relevant here, then it probably will also affect some other parts of SquidLib. Some parts might see noticeable improvement from chaining method calls, so this could still be useful to do, just probably not in a rarely-called toString() method.

[tommyettinger]

OK, I found your earlier issue on another repo that did get some discussion and benchmarking, https://github.com/influxdata/influxdb-java/issues/425 . I'm still wondering what the best technique is here to handle chaining of method calls in general, since SquidLib's GreasedRegion class can often be chain-called and I think often is in practice. I'll make the changes you suggested to improve the general style of the code, and I'll also look for places that are more likely to see a performance gain from this.