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

Peter Levart peter.levart at gmail.com
Wed Nov 30 15:31:47 UTC 2016

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 better or something, along with setIdentifierStart()?

I borrowed the name from java.io.InputStream::mark() which likewise "remembers" current read position of the stream. The InputStream uses the remembered position so it can reset() current position to it, while the parser uses the remembered position so it can return the subsequence of input from markToCurrent().

Also, "skipIdentifier" is a bit confusing, perhaps that means advanceToNextIdentifier?

The semantics of the method is to parse the following input as an identifier, but not give anything back as a result besides moving current position past the parsed identifier (hence the prefix "skip").
After this method returns, the parser is typically not positioned at the start of next identifier, but at some delimiter, so "advanceToNextIdentifier" would mean something completely different. I think the name is right for what it means.

Regards, Peter

On Wed, Nov 30, 2016 at 6:41 AM, Claes Redestad <claes.redestad at oracle.com <mailto:claes.redestad at oracle.com>> wrote: +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/8035424SignatureParser.performance/webrev.03/ <http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/8035424SignatureParser.performance/webrev.03/> Regards, Peter

More information about the core-libs-dev mailing list