> On 27 Jul 2016, at 19:36, Claes Redestad <claes.redes...@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. Paul. > >> >> Note that this has a minor side-effect that invocations of the form >> Runtime.Version.parse("notANumber") will now throw NFE instead of IAE. I >> don't think that this is a problem since NFE extends IAE and the order that >> the exceptions will be checked is intentionally unspecified. > > Right, I considered the minor behavior difference and thought it would be > considered OK and in accordance with the specification, and unproblematic > since this API is as of yet unreleased. > > As an aside: I do think specifying both NFE and IAE to be thrown is > suspicious when it's not perfectly clear when which one is to be expected, > since it invites to wrong code with strict dependencies on which is currently > thrown when, even though it's unspecified, which would mean changing the > implementation (like in this patch) would have some possibility of messing > with compatibility. I wonder if we should simply specify IAE and drop NFE to > allow greater flexibility for future implementations? > > /Claes > >> >> Regards, >> iris >> (not a Reviewer) >> >> -----Original Message----- >> From: Claes Redestad >> Sent: Saturday, July 23, 2016 9:24 AM >> To: core-libs-dev@openjdk.java.net core-libs-dev >> Subject: RFR: 8162439: Runtime.Version.parse needs fast-path for major >> versions >> >> Hi, >> >> please review this patch to address a startup regression due to use of >> Runtime.Version.parse("8") etc in JarFile, as introduced by JDK-8150680. >> This solution introduce a fast-path in case of what appears to be a single >> number is sent to Runtime.Version.parse to avoid initializing >> Runtime.VersionBuilder: >> >> Webrev: http://cr.openjdk.java.net/~redestad/8162439/webrev.01/ >> Bug: https://bugs.openjdk.java.net/browse/JDK-8162439 >> >> Passes all existing tests. >> >> Thanks! >> >> /Claes >