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

2016-11-30 Thread Peter Levart
Hi Max, On 11/30/2016 01:44 PM, Max Kanat-Alexander wrote: For what it's worth, this looks good to me too. :-) The name "mark" is a bit confusing, as are all the methods with the word "mark" in it; I have to think for a moment about what each of them do. Perhaps identifierStart would be bette

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

2016-11-30 Thread Max Kanat-Alexander
For what it's worth, this looks good to me too. :-) The name "mark" is a bit confusing, as are all the methods with the word "mark" in it; I have to think for a moment about what each of them do. Perhaps identifierStart would be better or something, along with setIdentifierStart()? Also, "skipIden

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

2016-11-30 Thread Claes Redestad
+1 Found a reference to getNext in a comment that should be removed, no re-review required. /Claes On 2016-11-30 12:36, Peter Levart wrote: Here's a webrev incorporating both suggestions. All 73 java/jang/reflect jtreg tests are passing... http://cr.openjdk.java.net/~plevart/jdk9-dev/80354

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

2016-11-30 Thread Peter Levart
Hi Claes, Andrej, On 11/29/2016 11:53 PM, Claes Redestad wrote: http://cr.openjdk.java.net/~plevart/jdk9-dev/8035424_SignatureParser.performance/webrev.02/ What do you think of this one? Fair point. I think this is easier to follow and should do the trick just as nicely. What you have he

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

2016-11-29 Thread Andrej Golovnin
Hi Peter, I know it is matter of taste, but don't you think this: 269 for (char c = current(); 270c != ';' && c != '.' && c != '/' && c != '[' && 271c != ':' && c != '>' && c != '<' && 272!Character.isWhitespace(c); 273 c =

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

2016-11-29 Thread Claes Redestad
On 11/29/2016 02:30 PM, Peter Levart wrote: 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 StringB

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

2016-11-29 Thread Peter Levart
Hi Claes, On 11/29/2016 11:30 PM, Peter Levart wrote: http://cr.openjdk.java.net/~plevart/jdk9-dev/8035424_SignatureParser.performance/webrev.02/ Which loop do you prefer. The above one or the following? private void skipIdentifier() { char c; while (!((c = current()) =

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

2016-11-29 Thread Peter Levart
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/~pl

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

2016-11-29 Thread 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/

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

2016-11-29 Thread Peter Levart
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