On 2016-06-27 11:18, Paul Sandoz wrote:
On 27 Jun 2016, at 10:39, Claes Redestad <claes.redes...@oracle.com> wrote:
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.
Ok. I was hoping consolidation of Optional production would compensate.
- 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?
Yes, that is ok.
-
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.
Drat, i missed the Integer.valueOf. I suppose one could change the types in
VersionProps to be Optional<String> values, then the semantics would be a
little clearer.
Mandy commented off-list yesterday about moving the conversion from
String to Optional<String> into VersionProps, which I think meshes well
with your suggestion:
http://cr.openjdk.java.net/~redestad/8160000/webrev.4/
(I still don't think calculating the number of dots is worth our while,
though)
Does this please?
/Claes
Paul.
