RFR: 8170467: (reflect) Optimize SignatureParser's use of StringBuilders

[Claes Redestad]

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.

>
> 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?)

/Claes

> Regards, Peter
>
>
> On 11/29/2016 03:03 PM, Claes Redestad wrote:
>> Hi Andrej,
>>
>> On 2016-11-29 14:39, Andrej Golovnin wrote:
>>> Hi Claes,
>>>
>>> 76     public static final int CLASS_NAME_SB_SIZE = 48;
>>>
>>> Why is this constant public?
>>
>> Good catch, made it private.
>>
>>> Btw. for our product this value is too small. We have packages with
>>> longer names. I would use 64. :-)
>>
>> There's no one size fits all: something around 48 is likely to
>> help most common cases, while not noticeably penalizing
>> shorter names. I'd consider it a bad move to regress apps
>> with short-to-normal names to get a small gain on apps with
>> very long names
>>
>> Thanks!
>>
>> /Claes
>>
>>>
>>> Best regards,
>>> Andrej Golovnin
>>>
>>> On Tue, Nov 29, 2016 at 2:18 PM, Claes Redestad
>>> <claes.redestad at oracle.com> wrote:
>>>> Hi,
>>>>
>>>> please review this patch provided by Max Kanat-Alexander[1] to improve
>>>> performance of sun.reflect.generics.parser.SignatureParser by reducing
>>>> number of StringBuilders created and the rate at which they are 
>>>> resized
>>>> during typical usage.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~redestad/8170467/webrev.01/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170467
>>>>
>>>> Thanks!
>>>>
>>>> /Claes
>>>>
>>>> [1]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/045046.html 
>>>>
>>
>