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

[Claes Redestad]

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/8170467_SignatureParser/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/8035424_SignatureParser.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