[PATCH] SignatureParser performance problems due to excessive StringBuilder usage

[Max Kanat-Alexander]

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<StringBuilder> 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