On 4/22/2014 6:12 PM, Vladimir Kozlov wrote:
On 4/22/14 4:42 PM, Alejandro E Murillo wrote:
On 4/21/2014 1:18 PM, Vladimir Kozlov wrote:
Hi Alejandro,
I don't think we need to rename make/hotspot_version file. It is still
used to set JVM's version string and not JDK's version.
Next assert message is not consistent with previous messages which use
"vm", I think it should be "vm" here too:
DEBUG_ONLY(assert_digits(vm_build_num, offset, "wrong JDK build
number"));
we do not have hotspot build number anymore, so I think we should not
mention hotspot build numberls
Can we simple say "wrong build number"?
sounds good!
So you don't want update build number in make/*_version files? ;)
Yes, I see in your example of JPRT build VM does not have build number
anymore.
it's gone!
Abstract_VM_Version::jvm_version() should include micro version. See
JVM_GetVersionInfo() in jvm.cpp and jvm_version_info in
jdk/src/share/javavm/export/jvm.h.
I added that here, is that what you are referring?
http://cr.openjdk.java.net/~amurillo/9/8030011/src/share/vm/runtime/vmStructs.cpp.udiff.html
http://cr.openjdk.java.net/~amurillo/9/8030011/src/share/vm/runtime/vm_version.hpp.udiff.html
No, I mean next code should encode micro version too:
unsigned int Abstract_VM_Version::jvm_version() {
return ((Abstract_VM_Version::vm_major_version() & 0xFF) << 24) |
((Abstract_VM_Version::vm_minor_version() & 0xFF) << 16) |
+ ((Abstract_VM_Version::vm_micro_version() & 0xFF) << 8) |
(Abstract_VM_Version::vm_build_number() & 0xFF);
}
you are right. I recall having added that earlier :(
it's back int the webrev
Use corresponding test in jdk for testing of these changes:
jdk/test/sun/misc/Version/Version.java
yeah, I run that test as part of jprt full builds,
That test handles both JDK and Hotspot express versions
Good.
jvm.h: Next comment is not accurate:
+ /* VM version string: JDK version string */
If we build VM separately (for example, in JPRT) VM version will not
be JDK version in which VM is installed. It will take numbers either
from passed make parameters or from make/hotspot_version. I think it
should say:
+ /* VM version string follows the JDK release version naming
convention
+ * <major>.<minor>.<micro>-bxx[-<identifier>][-<debug_flavor>]
Based on your examples [-<identifier>][-<debug_flavor>] is still used
so it should be reflected in the comment.
Let me make that explicit.
Don't remove next comments from vm_version.cpp but fix it ("follow the
JDK release"):
-// HOTSPOT_RELEASE_VERSION must follow the release version naming
convention
-// <major_ver>.<minor_ver>-b<nn>[-<identifier>][-<debug_target>]
ok
You did not show default VM version example when VM is built locally
by engineer.
It will be similar to the hotspot only jprt build. There will be a
mismatch,
I tested by dropping the resulting libjvm into a promoted build, so it
looks like this:
java version "1.9.0-ea"
Java(TM) SE Runtime Environment (build 1.9.0-ea-b01)
Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-internal-debug, mixed
mode)
okay
Please test that correct version string is constructed when you build
VM using make/build.sh, for example 'sh make/build.sh debug LP64=1'
Haven't tested that, thanks for pointing that out.
thank you
Next comment change in buildtree.make is not correct because
HOTSPOT_RELEASE_VERSION make parameter does not include
HOTSPOT_BUILD_VERSION:
-# HOTSPOT_RELEASE_VERSION - <major>.<minor>-b<nn> (11.0-b07)
+# HOTSPOT_RELEASE_VERSION - JRE_RELEASE_VERSION plus
HOTSPOT_BUILD_VERSION
see JPRT logs where HOTSPOT_BUILD_VERSION is set separately.
let me check that again
I think next change in make/defs.make is not safe (removing make
parameter) due to complexity of our builds:
-MAKE_ARGS += HOTSPOT_RELEASE_VERSION=$(HOTSPOT_RELEASE_VERSION)
I checked the code, and HOTSPOT_RELEASE_VERSION is only used in
vm_version.cpp,
so I think is fine to remove it. Note that if we keep it there, since
the JDK version string
sometimes might have time stamps, it may affect ccache, that's why at
some point they
separated the options for vm_version.cpp from the options for other
components. See this
comment on vm,make:
# This is VERY important! The version define must only be supplied to
vm_version.o
# If not, ccache will not re-use the cache at all, since the version
string might contain
# a time and date.
I was concern that it will not be passed into nested make so that, for
example, buildtree.make will not get it.
I see. I don't think it's needed
I think I incorporated all the changes David, John and you suggested and
started some sanity testing;
Here's is the latest webrev:
http://cr.openjdk.java.net/~amurillo/9/8030011/
Please review (don't forget to refresh each file on your browser) and
let me know if I missed anything.
thanks guys!
--
Alejandro
