This is a startup optimization, not a bootstrap issue: the Multi-release
feature touches Runtime.version() the first time a jar file is loaded,
so we'd like to defer lambda usage from this code since a great number
of small applications will take a relatively big hit.
/Claes
On 2016-06-27 16:54, Remi Forax wrote:
I'm not sure.
The lazy initialization code already defers the init of the version to (maybe)
a time where lambdas are available.
Rémi
----- Mail original -----
De: "Paul Sandoz" <paul.san...@oracle.com>
Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Lundi 27 Juin 2016 16:41:02
Objet: Re: RFR: 8160000: Runtime.version() cause startup regressions in 9+119
On 27 Jun 2016, at 16:36, Remi Forax <fo...@univ-mlv.fr> wrote:
Hi Claes,
Change looks good to me :)
just a minor nit
Optional<Integer> buildNumber = build.isPresent() ?
Optional.of(Integer.valueOf(build.get())) :
Optional.empty();
can be re-written
Optional<Integer> buildNumber = build.map(Integer::parseInt);
I presumed lambda’s were off limits for this section of code.
Paul.
BTW, i don't know why you're using Integerr.parseInt() when for the build
numbers in Version.parse() but Integer.valueOf() in version().
regards,
Rémi
----- Mail original -----
De: "Claes Redestad" <claes.redes...@oracle.com>
À: "Paul Sandoz" <paul.san...@oracle.com>
Cc: core-libs-dev@openjdk.java.net
Envoyé: Lundi 27 Juin 2016 16:16:45
Objet: Re: RFR: 8160000: Runtime.version() cause startup regressions in
9+119
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.
