RFR: 8162439: Runtime.Version.parse needs fast-path for major versions

Paul Sandoz paul.sandoz at oracle.com
Fri Jul 29 07:39:00 UTC 2016


> On 29 Jul 2016, at 05:15, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi Paul,
> 
> On 07/28/2016 02:55 PM, Paul Sandoz wrote:
>>> On 27 Jul 2016, at 19:36, Claes Redestad <claes.redestad at oracle.com> wrote:
>>> 
>>> On 07/25/2016 08:01 PM, Iris Clark wrote:
>>>> Hi, Claes.
>>>> 
>>>>> Webrev: http://cr.openjdk.java.net/~redestad/8162439/webrev.01/
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8162439
>>>> I think that this change looks good.  We provide a shortcut for the common case where only the major version number is of interest without introducing a new API.
>>> Thanks! Any reviewer want to give this the go-ahead?
>> Looks ok.
>> 
>> I suppose you could do:
>> 
>>   boolean isSimpleNumber(String s) {
>>     for (int i = 0; i < s.length(); i++) {
>>       char c = s.get(i);
>>       if (c < ((i > 0) ? ‘0’ : ‘1’) || c > ‘9’)
>>           return false;
>>     }
>>     return true;
>>   }
>> 
>> 
>>   if (isSimpleNumber(s)) {
>>     ...
>>   } else {
>>     return VersionBuilder.parse(s);
>>   }
>> 
>> then let VersionBuilder.parse throw errors in incorrectly formatted version strings.
> 
> sounds reasonable. It'd still change the behavior for the empty string from IAE to NFE, but only having to do one pass over the input string is nice.
> 

Hmm… i am inclined to agree because i am not sure given the pattern matching that one can reliably produce an NFE as anticipated in the JavaDoc.


> Another reasonable comment I got offline was that this patch splits the parsing into two places, which is hacky. Since what we really want to avoid is to eagerly load the Version string patterns (pulling in large parts of j.u.regex), we could inline the parsing code into Runtime.Version.parse, rename VersionBuilder to VersionPattern and access the constants therein from the parse method to allow lazy initialization. The overall result is arguably cleaner:
> 
> http://cr.openjdk.java.net/~redestad/8162439/webrev.02/
> 

Yes, much better. I was thinking similar thoughts about the split after i sent my previous email.

Paul.


More information about the core-libs-dev mailing list