[PATCH] SignatureParser performance problems due to excessive StringBuilder usage (original) (raw)

Max Kanat-Alexander mkanat at google.com
Tue Nov 29 12:16:01 UTC 2016

On Tue, Nov 29, 2016 at 7:08 AM, Claes Redestad <claes.redestad at oracle.com> wrote:

Hi and welcome!

Thanks! :-)

I've filed https://bugs.openjdk.java.net/browse/JDK-8170467 to track this.

Great, thanks. :-)

Introducing a parseIdentifierInto to avoid repeated creation of StringBuilder/String pairs seems like a no-brainer (my guess is this is the largest allocation reduction here?), and although I have a bit of a knee-jerk reaction against the magic number for the pre-sized builder it doesn't seem like an outrageous default for package.Class specifiers in the current ecosystem.

Yeah, the two biggest reducers are the pre-sizing and parseIdentifierInto.

Noting that there's no nested StringBuilder use in this code "pooling" them would be trivial (a single ThreadLocal which we setLength(0) after toString() would do), which could reduce allocations further at the cost of a very slight retained footprint increase and some loss of cache locality. It'd be interesting to see numbers on this, but I won't insist. :-)

Yeah, I supposed I'd also have to then be concerned about pathologically long identifiers which cause us to hold on to some huge StringBuilder forever, which seems like it would lead into cache management, and so forth and so forth. I actually think that the whole class could use some refactoring in general, but I wanted to keep this change focused on just one simple fix for the performance issue that I found.

Nit: the use of break-to-label in a non-nested while-loop seems awkward to me.

Yeah, agreed; it has to be done because we're in a switch statement and otherwise we would just be ending the case.

-Max

More information about the core-libs-dev mailing list