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

[Claes Redestad]

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.

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/


Thanks!

/Claes