RFR: 8160000: Runtime.version() cause startup regressions in 9+119

Claes Redestad claes.redestad at oracle.com
Mon Jun 27 08:39:27 UTC 2016


Hi Paul,

On 2016-06-27 10:07, Paul Sandoz wrote:
>
>> On 26 Jun 2016, at 21:55, Claes Redestad <claes.redestad at 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.
>


More information about the core-libs-dev mailing list