RFR: 8035424: (reflect) Performance problem in sun.reflect.generics.parser.SignatureParser (original) (raw)

Claes Redestad claes.redestad at oracle.com
Tue Nov 29 21:58:24 UTC 2016

Hi Peter,

On 11/29/2016 01:28 PM, Peter Levart wrote:

Hi Claes,

On 11/29/2016 06:34 PM, Claes Redestad wrote: Hi Peter,

On 11/29/2016 08:57 AM, Peter Levart wrote: Hi,

What about not using StringBuilder at all? http://cr.openjdk.java.net/~plevart/jdk9-dev/8170467SignatureParser/webrev.01/

your patch idea is rather clever but might prove to be better across the board with no need for any magic numbers, which is nice. It also improves the parseIdentifier() method - no double copying of chars as done when using StringBuilder.

Yes, I didn't suggest otherwise. :-)

There's also some mockery in getNext()/current()/advance() that uses exceptions for control flow which can be fixed while changing this part of code. The asserts in getNext()/current()/advance() could even fail if getNext()/advance() was called after already returning EOI (== character ':')... (mockery :D) Unfortunately I've already pushed Max's patch, but there's also https://bugs.openjdk.java.net/browse/JDK-8035424 which asks for getting rid of the exceptional flow using a suggestion similar to yours, and I don't mind sponsoring another go at this. I suggest we move ahead with your patch using that bug ID. (getNext - and matches - appear unused and could be removed rather than fixed?) Right, do you want just removal of exceptions or the whole thing? I reduced copying further. The 'input' field is a char[], but could simply be a String. No need to extract input String's chars into an array 1st: http://cr.openjdk.java.net/~plevart/jdk9-dev/8035424SignatureParser.performance/webrev.01/

I think pushing this as 8035424 is OK.

I know I just told you to remove it, but wouldn't the slightly awkward for loops read better as:

char c = current(); while (!(...)) { c = getNext(); }

Thanks!

/Claes

More information about the core-libs-dev mailing list