Hi Alejandro,
I looked at the hotspot and JDK changes.
On 17/06/2015 8:55 AM, Alejandro E Murillo wrote:
Please review these changes:
Bug: https://bugs.openjdk.java.net/browse/JDK-8087202
Webrev: http://cr.openjdk.java.net/~amurillo/9/8087202
hotspot/make/Makefile
+ # VERSION_PATCH Security number for version (e.g. 0)
Comment is wrong.
---
hotspot/src/share/vm/prims/jvm.cpp
3659 info->patch_version = 0;
Why is this hard-wired to zero? Surely this should be a build variable.
---
hotspot/src/share/vm/prims/jvm.h
jdk/src/java.base/share/native/include/jvm.h
These two files should be identical, but now have different comments in
places.
1188 unsigned int patch_version : 8;
1215 unsigned int patch_version : 8;
In each case you removed two 8 bit fields and added one 8 bit field so
your bitfields no longer add up to 32 (8+8+16). But these structs seem
obsolete (due to no hotspot-express) even before the new version
changes. So I think there is still a bit of follow up work to do here.
---
hotspot/src/share/vm/runtime/java.hpp
110 _partially_initialized(false),
Isn't the whole partially vs fully initialized issue dead now? Else why
delete the other code pertaining to that?
---
jdk/src/java.base/share/classes/sun/misc/Version.java.template
* @since JDK9
Is that the right format? I would have expected @since 9.
248 private static native boolean getJvmVersionInfo();
Does that method still need to return a boolean ? See next comment
---
jdk/src/java.base/share/native/libjava/Version.c
50 if (!JDK_InitJvmHandle()) {
51 JNU_ThrowInternalError(env, "Handle for JVM not found for
symbol lookup");
52 return JNI_FALSE;
53 }
54 func_p = (GetJvmVersionInfo_fp)
JDK_FindJvmEntry("JVM_GetVersionInfo");
55 if (func_p == NULL) {
56 return JNI_FALSE;
57 }
this seems dead code now and so the method can't fail or throw an exception.
Thanks,
David
-----
These are intended to:
(1) Add support for the patch field of the new version string format
(2) Remove unused fields remaining from the old version string format
(3) Patch some jaxp initialization code to be able to handle the new
version string.
this will be pushed under a different bug number, but is required
to be able to run and pass all the tests associated with
"-testset hotspot"
(for JFR tests the VM can't be started without that).
These will be pushed under this sub-task:
https://bugs.openjdk.java.net/browse/JDK-8098588
(4) Additional notes:
(a) Some pieces of code, only necessary for 1.5 or older, were removed
(b) update_version and special_update_version were removed
(c) The code to parse the version string in
jdk/test/sun/misc/Version/Version.java
will probably change when the Version.java class is available.
(d) As with the changes for 8085822, this is all being pushed to [1] and
then later to jdk9/dev
(e) These changes should address most, if not all, of the issues raised
during
the code review for JDK-8085822 (see also
https://bugs.openjdk.java.net/browse/JDK-8087203)
(f) All builds and tests pass for "-testset hotspot".
[1] http://hg.openjdk.java.net/verona/stage
Thanks
