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
<[email protected]> 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
