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

[Claes Redestad]

On 2016-06-27 22:11, Iris Clark wrote:
> Hi, Claes.
>
> [ Sorry for the premature send, my keyboard started interpreting my shortcuts
> in unexpected ways. ]
>
>> http://cr.openjdk.java.net/~redestad/8160000/webrev.5/
>
> Nice bugid.

I can hardly believe my luck! :-)

>
> Overall, this change looks good.  I just have a few concerns.
>
> Have you built this forcing alternative build numbers using the makefile
> "--with-version-<...>" options?  For JDK 9 builds, the default version number
> is only "9".  You should make sure that versions "9.0.X" and "9.0.0.X" for
> arbitrary value of X work as expected.

Done.

>
> Runtime.java:  I wonder if unmodifiablelist() should be invoked at new line
> 1090 instead of 941?  Depending on the caller to provide an unmodifiable list
> without additional verification seems like a potential problem.  If you move
> the invocation, then you may want to consider reducing the arguments of
> Version() at lines 1086-1089 to the single VersionProps.
>
> I expect to address the following outstanding issues in the coming weeks.  I
> don't think that they will impact your changes, but I leave it for you to
> judge:
>
>      8156711: Runtime.Version.version should be an int[] instead of a
>        List<Integer>
>      https://bugs.openjdk.java.net/browse/JDK-8156711
>
> This is an implementation change that was suggested during review.  The
> thought was that using an int[] is more efficient for space and comparisons.
> If you believe that there is benefit to adding this change to yours, feel free
> to take on the bug.  Otherwise, assuming that there is still benefit to making
> this change, I'll take care of it after your changes are in a promotion.

Seems trivial enough, but I think the benefit of this is marginal at 
best unless we expect Runtime.Version to be used extensively in 
performance critical code. For startup it may even be an ever so slight 
negative if it means more code to parse the built-in version string.

>
>      8156907: Runtime.Version.{major(),security()} return value of 0 may be
>        ambiguous
>      https://bugs.openjdk.java.net/browse/JDK-8156907
>
> After careful consideration, this issue will be resolved by changing the
> parse() implementation to allow trialing zero elements (now in
> VersionBuilder.parse()).  As an example of the target behavior,
> v = Runtime.Version.parse("9.0.0") will not throw an IAE and v.toString()
> will return "9".  Specification adjustments will be needed.  Since
> VersionBuilder does not change the parse() implementation, I believe that
> there won't be any problems.

Good.

I don't think there'll be any conflict with this change, and as I was 
testing I also made sure that X=0 works well since the build scripts 
already handles shortening this to "9".

To accommodate your change request w.r.t. unmodifiableList above I took 
the liberty of cleaning up VersionBuilder.parse() a bit, though. Hope 
you don't mind:

http://cr.openjdk.java.net/~redestad/8160000/webrev.6/

Thanks!

/Claes

>
> Thanks,
> iris
>