On 11/29/2016 10:58 PM, Claes Redestad wrote:
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();
}
They would if c = getNext(); was equivalent to { advance(); c =
current(); }, but it was not. It was defined as: { c = current();
advance(); }.
I could re-introduce a different getNext() with the desired behavior,
but here's another variant with a more "streaming" approach which tries
to re-use the logic for parsing the identifier:
http://cr.openjdk.java.net/~plevart/jdk9-dev/8035424_SignatureParser.performance/webrev.02/
What do you think of this one?
Regards, Peter
Thanks!
/Claes
