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

Max Kanat-Alexander mkanat at google.com
Wed Nov 30 23:43:33 UTC 2016

Okay. As a reader, "skip" to me means "ignore" or something like that, and "mark" isn't inherently meaningful, although I figured it out by the time I read the whole file.

On Wed, Nov 30, 2016 at 10:31 AM, Peter Levart <peter.levart at gmail.com> wrote:

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_ _> 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/8035424Signatu reParser.performance/webrev.03/

Regards, Peter

More information about the core-libs-dev mailing list