Hi Paul,
On 2016-06-27 10:07, Paul Sandoz wrote:
On 26 Jun 2016, at 21:55, Claes Redestad <claes.redes...@oracle.com> wrote:
Hi,
9+119 changed java.util.regex to initialize java.lang.invoke early, causing a
number of easily reproducible startup regressions.
This patch uses the fact that we already maintain the version string
constituents during build time to simplify creation of the
java.lang.Runtime.version().
Webrev: http://cr.openjdk.java.net/~redestad/8160000/webrev.3/
Bug: https://bugs.openjdk.java.net/browse/JDK-8160000
Since getting Runtime.version() now does not have to touch java.util.regex
classes we end up slightly ahead of previous builds for applications which does
not use regular expressions.
Thanks!
Looks good.
Thanks for reviewing this!
- Perhaps it’s worth pre-sizing the array list exactly by counting the ‘.’
before processing? or is 4 always pre-sized exactly?
I figured saving a few bytes in the best case by adding more bytecode
and an extra scan of the string would just shift costs around.
- add an assert to check Version produced by version() is the same as that
produced the previous way parsing the sys prop
I actually had that in an earlier version of the patch but found that
that is already tested by test/java/lang/Runtime/Version/Basic.java -
testVersion and was thus redundant. Do you agree?
-
957 if (!VersionProps.VERSION_PRE.isEmpty()) {
958 pre = Optional.of(VersionProps.VERSION_PRE);
959 } else {
960 pre = Optional.empty();
961 }
Encapsulate that and the other two into a separate method e.g. optionalOfEmpty
then do:
version = new Version(…
optionalOfEmpty(VersionProps.VERSION_PRE),
…
);
Yes, it'd have been neater if not for the fact that VERSION_BUILD is to
be parsed as an Integer, which complicates it a bit. Maybe it is still
an improvement.
Thanks!
/Claes
Paul.
