Re: [verona.stage] RFR 8139986: Store debug level in java.vm.debug and conditionally print in "java -version"
Thanks David! Alejandro On 11/3/2015 8:52 PM, David Holmes wrote: Looks good to me! Thanks, David On 4/11/2015 3:45 AM, Alejandro E Murillo wrote: Please review these changes: bug: https://bugs.openjdk.java.net/browse/JDK-8139986 Webrev: http://cr.openjdk.java.net/~amurillo/9/8139986/ Background: These changes introduce a new system property named "jdk.debug" intended to identify the type of the build. The build system has already been modified (see [1]) to provide the build type through the "--with-debug-level" configure option, and to remove that info from the (new) version string and consequently from the "java.version" and "java.vm.version" system properties. Here, the configure debug level is used to initialize the value of the "jdk.debug" system property. There are also changes to adapt any code that relied on the value of those version properties to determine the build type. They were changed to use this new property. The Launcher output was also modified to look as follows: jdk.debug = (“*foo*” != “release”) $java -version java version "9-ea" Java(TM) SE Runtime Environment (*foo *build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (*foo *build 9-ea+88, mixed mode) jdk.debug = “release”: (no change) $java -version java version "9-ea" Java(TM) SE Runtime Environment (build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (build 9-ea+88, mixed mode) All this will be described and updated in the JEP-223 doc [2] shortly. [1] https://bugs.openjdk.java.net/browse/JDK-8139951 [2] http://openjdk.java.net/jeps/223 Thanks -- Alejandro
Re: [verona.stage] RFR 8139986: Store debug level in java.vm.debug and conditionally print in "java -version"
On 11/3/2015 3:42 PM, Daniel D. Daugherty wrote:
On 11/3/15 10:45 AM, Alejandro E Murillo wrote:
Please review these changes:
bug: https://bugs.openjdk.java.net/browse/JDK-8139986
Webrev: http://cr.openjdk.java.net/~amurillo/9/8139986/
jdk/src/java.base/share/classes/sun/misc/Version.java.template
nit: L103: if (jdk_debug_level.startsWith("release") )
Please delete extra space between right parens.
will fix that
Also, looks like other single line if-statements in
this file uses this format:
if (expr) {
statement;
}
right, will also fix that
thanks Dan!
Alejandro
jdk/test/lib/testlibrary/jdk/testlibrary/Platform.java
No comments.
hotspot/make/aix/makefiles/vm.make
No comments.
hotspot/make/bsd/makefiles/vm.make
No comments.
hotspot/make/linux/makefiles/vm.make
No comments.
hotspot/make/solaris/makefiles/vm.make
No comments.
hotspot/make/windows/makefiles/defs.make
No comments.
hotspot/make/windows/makefiles/vm.make
No comments.
hotspot/make/windows/projectfiles/common/Makefile
No comments.
hotspot/src/share/vm/runtime/arguments.cpp
No comments.
hotspot/src/share/vm/runtime/statSampler.cpp
No comments.
hotspot/src/share/vm/runtime/vm_version.cpp
nit L68 #error DEBUG_LEVEL must be defined
Please delete the extra space before "must be..."
hotspot/src/share/vm/runtime/vm_version.hpp
No comments.
hotspot/test/testlibrary/jdk/test/lib/Platform.java
No comments.
Thumbs up. If you fix the minor style issues above,
I don't need to see another webrev.
Dan
Background:
These changes introduce a new system property named "jdk.debug"
intended to
identify the type of the build. The build system has already been
modified (see [1])
to provide the build type through the "--with-debug-level" configure
option,
and to remove that info from the (new) version string and
consequently from the "java.version" and "java.vm.version" system
properties.
Here, the configure debug level is used to initialize the value of
the "jdk.debug" system
property. There are also changes to adapt any code that relied on the
value of those version
properties to determine the build type. They were changed to use
this new property.
The Launcher output was also modified to look as follows:
jdk.debug = (“*foo*” != “release”)
$java -version
java version "9-ea"
Java(TM) SE Runtime Environment (*foo *build 9-ea+88)
Java HotSpot(TM) 64-Bit Server VM (*foo *build 9-ea+88,
mixed mode)
jdk.debug = “release”: (no change)
$java -version
java version "9-ea"
Java(TM) SE Runtime Environment (build 9-ea+88)
Java HotSpot(TM) 64-Bit Server VM (build 9-ea+88, mixed mode)
All this will be described and updated in the JEP-223 doc [2] shortly.
[1] https://bugs.openjdk.java.net/browse/JDK-8139951
[2] http://openjdk.java.net/jeps/223
Thanks
--
Alejandro
Re: [verona.stage] RFR 8139986: Store debug level in java.vm.debug and conditionally print in "java -version"
Thanks Magnus! Alejandro On 11/3/2015 12:24 PM, Magnus Ihse Bursie wrote: Hi Alejandro, On 2015-11-03 18:45, Alejandro E Murillo wrote: Please review these changes: bug: https://bugs.openjdk.java.net/browse/JDK-8139986 Webrev: http://cr.openjdk.java.net/~amurillo/9/8139986/ Looks good to me. /Magnus Background: These changes introduce a new system property named "jdk.debug" intended to identify the type of the build. The build system has already been modified (see [1]) to provide the build type through the "--with-debug-level" configure option, and to remove that info from the (new) version string and consequently from the "java.version" and "java.vm.version" system properties. Here, the configure debug level is used to initialize the value of the "jdk.debug" system property. There are also changes to adapt any code that relied on the value of those version properties to determine the build type. They were changed to use this new property. The Launcher output was also modified to look as follows: jdk.debug = (“*foo*” != “release”) $java -version java version "9-ea" Java(TM) SE Runtime Environment (*foo *build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (*foo *build 9-ea+88, mixed mode) jdk.debug = “release”: (no change) $java -version java version "9-ea" Java(TM) SE Runtime Environment (build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (build 9-ea+88, mixed mode) All this will be described and updated in the JEP-223 doc [2] shortly. [1] https://bugs.openjdk.java.net/browse/JDK-8139951 [2] http://openjdk.java.net/jeps/223 Thanks -- Alejandro
[verona.stage] RFR 8139986: Store debug level in java.vm.debug and conditionally print in "java -version"
Please review these changes: bug: https://bugs.openjdk.java.net/browse/JDK-8139986 Webrev: http://cr.openjdk.java.net/~amurillo/9/8139986/ Background: These changes introduce a new system property named "jdk.debug" intended to identify the type of the build. The build system has already been modified (see [1]) to provide the build type through the "--with-debug-level" configure option, and to remove that info from the (new) version string and consequently from the "java.version" and "java.vm.version" system properties. Here, the configure debug level is used to initialize the value of the "jdk.debug" system property. There are also changes to adapt any code that relied on the value of those version properties to determine the build type. They were changed to use this new property. The Launcher output was also modified to look as follows: jdk.debug = (“*foo*” != “release”) $java -version java version "9-ea" Java(TM) SE Runtime Environment (*foo *build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (*foo *build 9-ea+88, mixed mode) jdk.debug = “release”: (no change) $java -version java version "9-ea" Java(TM) SE Runtime Environment (build 9-ea+88) Java HotSpot(TM) 64-Bit Server VM (build 9-ea+88, mixed mode) All this will be described and updated in the JEP-223 doc [2] shortly. [1] https://bugs.openjdk.java.net/browse/JDK-8139951 [2] http://openjdk.java.net/jeps/223 Thanks -- Alejandro
Re: [verona.stage] RFR 8087203: Adapt Version.java.template to the JEP-223 new version string format
Thanks Iris Alejandro On 9/17/2015 4:17 PM, Iris Clark wrote: Hi, Alejandro. This cleanup looks good to me. Thanks, iris (not a JDK 9 Reviewer) -Original Message- From: Alejandro E Murillo Sent: Wednesday, September 16, 2015 11:04 AM To: core-libs-dev@openjdk.java.net Cc: verona-...@openjdk.java.net Subject: [verona.stage] RFR 8087203: Adapt Version.java.template to the JEP-223 new version string format Please review these changes: Bug: https://bugs.openjdk.java.net/browse/JDK-8087203 Webrev: http://cr.openjdk.java.net/~amurillo/9/8087203/ The actual code changes for this bug (8087203) were entered along with the ones for 8087202. The changes here are just fixing some javadoc comments in that template Thanks -- Alejandro -- Alejandro
Re: [verona.stage] RFR 8134365: Test test/sun/misc/Version/Version.java should follow Verona rules for trailing zeros
On 9/16/2015 6:44 PM, Mandy Chung wrote: On Sep 16, 2015, at 11:23 AM, Alejandro E Murillo wrote: Please review this change: Bug: https://bugs.openjdk.java.net/browse/JDK-8134365 Webrev: http://cr.openjdk.java.net/~amurillo/9/8134365/ This change modifies the toString() method of: test/sun/misc/Version/Version.java so that it doesn't include trailing zeros when the some version fields are not defined. For example, if the version is 9-ea+b2, "toString()" should not return 9.0.0.0-ea+b2 This looks okay to me. Mandy Thanks Mandy -- Alejandro
[verona.stage] RFR 8134365: Test test/sun/misc/Version/Version.java should follow Verona rules for trailing zeros
Please review this change: Bug: https://bugs.openjdk.java.net/browse/JDK-8134365 Webrev: http://cr.openjdk.java.net/~amurillo/9/8134365/ This change modifies the toString() method of: test/sun/misc/Version/Version.java so that it doesn't include trailing zeros when the some version fields are not defined. For example, if the version is 9-ea+b2, "toString()" should not return 9.0.0.0-ea+b2 note that this class will eventually be modified to use the Java API to parse, validate, and compare version strings once it is available. That's being tracked under: https://bugs.openjdk.java.net/browse/JDK-8136651 Thanks -- Alejandro
[verona.stage] RFR 8087203: Adapt Version.java.template to the JEP-223 new version string format
Please review these changes: Bug: https://bugs.openjdk.java.net/browse/JDK-8087203 Webrev: http://cr.openjdk.java.net/~amurillo/9/8087203/ The actual code changes for this bug (8087203) were entered along with the ones for 8087202. The changes here are just fixing some javadoc comments in that template Thanks -- Alejandro
Re: [verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Sounds good Joe, I will push those soon. Thanks Alejandro On 6/19/2015 3:56 PM, huizhe wang wrote: Hi Alejandro, Alan's right about the JAXP version-check code. But since this is urgently needed, I agree you can push the change as is. I'll create a separate bug to re-examine version related code in jaxp. As Alan pointed out, it would be some clean-up. Thanks, Joe On 6/19/2015 9:53 AM, Alejandro E Murillo wrote: Hi Alan, just to you. didn't hear back from you, so I'll assume you are fine with the corrections. We want to get this into further testing so I'm going to push the changes cheers Alejandro On 6/18/2015 4:56 PM, Alejandro E Murillo wrote: Thanks Alan, see below On 6/18/2015 7:41 AM, Alan Bateman wrote: On 16/06/2015 23:55, 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 The implementation of isJavaVersionAtLeast in the JAXP classes look okay although I think this is code that could be removed. Joe Wang can confirm but I think it dates back to when the JAXP API was a standalone API and there was an attempt to keep the code in sync across major versions. I'll create a separate bug re-examine this as it looks like some clean-up can be done here. sounds good, in general, that code can be called a lot, so changes need to be carefully done as to avoid perf impact. So it might be better if that check can be removed Just on the comment "In JDK9 the version string was changed ..." will date quickly and would be nice to say that "In JDK 8 and older then it assumes 1.N and for JDK 9 and newer it assumes N. In sun.misc.Version.initVersions then InternalError instead of RuntimeException might be more appropriate as something is really broken if that happens. Indeed. Changed. Also as David pointed out, it shouldn't be @since JDK9 in the new method. This reminds me to check if the JEP says anything about @since tags because we already have quite a few @since 1.9. yeah, I had meant to double check that. Will change it to 9 In the Version.java test then the line with the pattern is really long, maybe that could be split up to make future side-by-side views easier to read. sure, will make it into several lines, based on components, like this: String jep223Pattern = "^([0-9]+)(\\.([0-9]+))?(\\.([0-9]+))?(\\.([0-9]+))?" + // $VNUM "(-([a-zA-Z]+))?(\\.([a-zA-Z]+))?" + // $PRE "(\\+([0-9]+))?" + // Build Number "(([-a-zA-Z0-9.]+))?$"; // $OPT see new webrev: http://cr.openjdk.java.net/~amurillo/9/8087202.v2/jdk/src/java.base/share/classes/sun/misc/Version.java.template.udiff.html Otherwise looks okay to me. great, thanks! Here's the new webrev (I tested these with JPRT, testsets hotspot and pit): http://cr.openjdk.java.net/~amurillo/9/8087202.v2/ Thanks! -- Alejandro
Re: [verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Hi Alan, just to you. didn't hear back from you, so I'll assume you are fine with the corrections. We want to get this into further testing so I'm going to push the changes cheers Alejandro On 6/18/2015 4:56 PM, Alejandro E Murillo wrote: Thanks Alan, see below On 6/18/2015 7:41 AM, Alan Bateman wrote: On 16/06/2015 23:55, 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 The implementation of isJavaVersionAtLeast in the JAXP classes look okay although I think this is code that could be removed. Joe Wang can confirm but I think it dates back to when the JAXP API was a standalone API and there was an attempt to keep the code in sync across major versions. I'll create a separate bug re-examine this as it looks like some clean-up can be done here. sounds good, in general, that code can be called a lot, so changes need to be carefully done as to avoid perf impact. So it might be better if that check can be removed Just on the comment "In JDK9 the version string was changed ..." will date quickly and would be nice to say that "In JDK 8 and older then it assumes 1.N and for JDK 9 and newer it assumes N. In sun.misc.Version.initVersions then InternalError instead of RuntimeException might be more appropriate as something is really broken if that happens. Indeed. Changed. Also as David pointed out, it shouldn't be @since JDK9 in the new method. This reminds me to check if the JEP says anything about @since tags because we already have quite a few @since 1.9. yeah, I had meant to double check that. Will change it to 9 In the Version.java test then the line with the pattern is really long, maybe that could be split up to make future side-by-side views easier to read. sure, will make it into several lines, based on components, like this: String jep223Pattern = "^([0-9]+)(\\.([0-9]+))?(\\.([0-9]+))?(\\.([0-9]+))?" + // $VNUM "(-([a-zA-Z]+))?(\\.([a-zA-Z]+))?" + // $PRE "(\\+([0-9]+))?" + // Build Number "(([-a-zA-Z0-9.]+))?$"; // $OPT see new webrev: http://cr.openjdk.java.net/~amurillo/9/8087202.v2/jdk/src/java.base/share/classes/sun/misc/Version.java.template.udiff.html Otherwise looks okay to me. great, thanks! Here's the new webrev (I tested these with JPRT, testsets hotspot and pit): http://cr.openjdk.java.net/~amurillo/9/8087202.v2/ Thanks! -- Alejandro
Re: [verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Hi David,
thanks for the review, see below
On 6/18/2015 1:40 AM, David Holmes wrote:
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.
cut and paste. Will fix
---
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.
good catch! That should be initialized from VM_Version as well
(was mimicking update version, but that was always zero for hotspot
which is not the case for patch). New webrev:
http://cr.openjdk.java.net/~amurillo/9/8087202.v2/hotspot/src/share/vm/prims/jvm.cpp.udiff.html
---
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.
will add the missing comments
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.
oh yes, looks like I need to keep the size of the whole structure,
to avoid any misalignment that may impact performance.
I thought about declaring patch to be short (16) instead,
but I ended up adding a padding member, reserved3, size 8,
to keep the structure size unchanged. I guess I could have changed
reserved2 to 24, let me know if there's any advantage using either of those.
New webrev:
http://cr.openjdk.java.net/~amurillo/9/8087202.v2/hotspot/src/share/vm/prims/jvm.h.udiff.html
---
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?
yeah, I should have removed the remaining uses of that.
It's gone now.
---
jdk/src/java.base/share/classes/sun/misc/Version.java.template
* @since JDK9
Is that the right format? I would have expected @since 9.
I meant to double check that. I changed it
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.
I thought about changing that, but I prefer if it's done
as a separate change for now. will file a follow up bug,
Are you ok with that?
New webrev here (I tested these with JPRT, testsets hotspot and pit):
http://cr.openjdk.java.net/~amurillo/9/8087202.v2/
Thanks
Alejandro
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
--
Alejandro
Re: [verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Thanks Alan, see below On 6/18/2015 7:41 AM, Alan Bateman wrote: On 16/06/2015 23:55, 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 The implementation of isJavaVersionAtLeast in the JAXP classes look okay although I think this is code that could be removed. Joe Wang can confirm but I think it dates back to when the JAXP API was a standalone API and there was an attempt to keep the code in sync across major versions. I'll create a separate bug re-examine this as it looks like some clean-up can be done here. sounds good, in general, that code can be called a lot, so changes need to be carefully done as to avoid perf impact. So it might be better if that check can be removed Just on the comment "In JDK9 the version string was changed ..." will date quickly and would be nice to say that "In JDK 8 and older then it assumes 1.N and for JDK 9 and newer it assumes N. In sun.misc.Version.initVersions then InternalError instead of RuntimeException might be more appropriate as something is really broken if that happens. Indeed. Changed. Also as David pointed out, it shouldn't be @since JDK9 in the new method. This reminds me to check if the JEP says anything about @since tags because we already have quite a few @since 1.9. yeah, I had meant to double check that. Will change it to 9 In the Version.java test then the line with the pattern is really long, maybe that could be split up to make future side-by-side views easier to read. sure, will make it into several lines, based on components, like this: String jep223Pattern = "^([0-9]+)(\\.([0-9]+))?(\\.([0-9]+))?(\\.([0-9]+))?" + // $VNUM "(-([a-zA-Z]+))?(\\.([a-zA-Z]+))?" + // $PRE "(\\+([0-9]+))?" + // Build Number "(([-a-zA-Z0-9.]+))?$"; // $OPT see new webrev: http://cr.openjdk.java.net/~amurillo/9/8087202.v2/jdk/src/java.base/share/classes/sun/misc/Version.java.template.udiff.html Otherwise looks okay to me. great, thanks! Here's the new webrev (I tested these with JPRT, testsets hotspot and pit): http://cr.openjdk.java.net/~amurillo/9/8087202.v2/ Thanks! -- Alejandro
[verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string
Please review these changes: Bug: https://bugs.openjdk.java.net/browse/JDK-8087202 Webrev: http://cr.openjdk.java.net/~amurillo/9/8087202 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 -- Alejandro
Re: RFR: JDK-8085822 JEP 223: New Version-String Scheme (initial integration)
On 6/10/2015 6:13 AM, Magnus Ihse Bursie wrote: On 2015-06-10 11:58, David Holmes wrote: Hi Magnus, Generally looks good - a few comments/queries below. In general, I believe most issues you found are valid. :-) However, as I said before in this thread, I'd like to see them resolved in the needed follow-up patches. The source code changes in Hotspot and JDK are inadequate to fully implement JEP-223, so these areas will need to be rewritten anyhow. Are you okay with resolving these issues later? On 9/06/2015 11:52 PM, Magnus Ihse Bursie wrote: Here's an updated webrev, which fixes the typos that were pointed out by reviewers: http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.02/ common/autoconf/version-numbers Shouldn't there be a DEFAULT_VERSION_XXX for each of the XXX parts of the version info? (Even if all zeroes presently.) So that's a tricky one. :-& The question here is, I think, does it make sense to update version-numbers when we do a security (9.0.1) or minor (9.1.0) release? Looking historically, the version-numbers file have not been changed for the 8u family. The only change was going from 8 to 9 when creating the new jdk9 forest. That was what I based my decition to only have the major number in the file. (The rest of the version string is set by configure flags when building, in Oracle case by the RE team.) If it makes sense to put the minor/security/patch numbers here, I won't oppose it, but I guess we have until the first such release to figure out what's best, and that likely includes discussion with RE and possibly other consumers in the community (RedHat etc). --- Looking at hotspot changes I can't convince myself that the new streamlined "version" variables will capture things like "fastdebug". Will that filter through via configure variables? The "fastdebug" label has been handled as a less valued token in the JEP-223 process. Right now there's no mention of it at all in the version string proposal in the JEP. As I understand it, Iris is about to propose an amendment which will just make fastdebug be a part of the OPT string. I'm not entirely happy with that myself, but that's the way it's decided. So wherever you can see the OPT string, you'll see the debug level tag. Currently, however, that's not how it is implemented in this patch. Since no decision was made on this (and it's still not formally decided), I had to pick a route to go forward. The current patch will instead put the "fastdebug" label as part of the PRE string (that's the reason for the pre-base and pre-debuglevel arguments to configure). If the planned changes goes into the JEP, this'll change to make the debuglevel tag a part of the OPT string instead. --- make/*/makefiles/vm.make Shouldn't the -DVERSION_XX=$(VERSION_XX) definitions be taken from the VERSION_CFLAGS in spec.gmk? (Or are you still allowing for a stand-alone hotspot build?) The hotspot JEP-223 work initially made by Alejandro kept support for stand-alone hotspot builds. I made additional changes on top of that, which still tried to cater for stand-alone builds. At this very moment I'm not sure if stand-alone hotspot builds are supposed to work or not, but I've tried to not change the situation with this patch. There are two future changes coming down the pipe: One is the additional JEP-223 work needed for Hotspot. I know Alejandro had plans for redesigning the API between Hotspot and the JDK, so no JDK version strings should be compiled into the JVM, but all of it extracted during an API in runtime. That would make most (all?) of the makefile changes in hotspot irrelevant. However, that implementation is not even started, so it's needed for the time being. There's already an API used by Hotspot to get JDK version values. I'm investigating using that and extend it if required. yes, there is a lot stuff in the hotspot code that should be removed (mostly in the makefiles) and I'm not against making those changes now, I just think they are somewhat out of the scope of this project. And should be done as individual RFEs or as part of the upcoming hotspot makefile refactoring. Alejandro The second change is the build-infra hotspot makefile rewrite. When that happens, stand-alone builds will definitely disappear, and if Hotspot still needs make support for version strings, then the logical choice is to use VERSION_CFLAGS. Ok? --- hotspot/src/share/vm/prims/jvm.h jdk/src/java.base/share/native/include/jvm.h I think this comment is way out of date: /* Build number is available only for RE build (i.e. JDK_BUILD_NUMBER is set to bNN) * It will be zero for internal builds. */ and can be completely removed. Even if still true, mention of an "RE build" has no place in OpenJDK sources. Yep, agree. Follow-up patch, right? --- hotspot/src/share/vm/runtime/java.cpp I think a lot of the modified code is obsolete post Hotspot Express - which
Re: JDK 9 RFR of JDK-8075551: Add tiered testing definitions to the jaxp repo
On 6/1/2015 6:15 PM, huizhe wang wrote: On 6/1/2015 3:16 PM, joe darcy wrote: Hello, Please review these changes to regularize the jaxp regression testing infrastructure with the JDK tiered testing policy. [1] JDK-8075551: Add tiered testing definitions to the jaxp repo http://cr.openjdk.java.net/~darcy/8075551.0/ In brief, testing tiers are defined (with an empty tier 1 for jaxp) and an empty problem list is added. The change looks good to me. When analogous changes are made in all the repositories, it will be possible to use a jtreg command like jtreg -exclude:ProblemList.txt [... other options...] :tier$N How does it work in JPRT, something like "-testset core:tier$2"? I'm also very interested on an answer to this thanks Alejandro Thanks, Joe for any repo. Thanks, -Joe [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-March/001991.html -- Alejandro
Re: Build failures on solaris
Thanks for the quick fix! Alejandro On 5/9/2014 1:28 PM, Eric McCorkle wrote: Pushed. You may now resume normal integration activities. On 05/09/14 15:12, Staffan Larsen wrote: Looks good. Many apologies. /Staffan On 9 maj 2014, at 21:08, Eric McCorkle wrote: The following patch will fix it: diff -r 7426549b1e3b src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c --- a/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c +++ b/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c @@ -1,4 +1,4 @@ -''/* +/* * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * On 05/09/14 15:01, Eric McCorkle wrote: The problem is a stray '' as the first two characters in the file. I have posted a patch that removes it to serviceability-dev. All we need is someone with Reviewer/committer rights to step in and apply it. On 05/09/14 14:48, Alejandro E Murillo wrote: Definitively a P1. This is also blocking this week hotspot snapshot: http://prt-web.us.oracle.com//archive/2014/05/2014-05-09-174238.amurillo.jdk9-hs-2014-05-09-jdk9-dev-control/logs/solaris_sparcv9_5.10-fastdebug.log.FAILED.log please fix ASAP Thanks Alejandro On 5/9/2014 12:16 PM, Eric McCorkle wrote: Bug created: https://bugs.openjdk.java.net/browse/JDK-8042859 This patch seems to be the culprit: changeset: 9908:7426549b1e3b tag: tip user:sla date:Fri May 09 12:06:13 2014 +0200 summary: 8039173: Propagate errors from Diagnostic Commands as exceptions in the attach framework On 05/09/14 14:08, Eric McCorkle wrote: I am currently seeing build failures on solaris, coming from within the JDK repo: /opt/jprt/products/P1/SS12u1/SS12u1/prod/bin/cc -DTRACING -DMACRO_MEMSYS_OPS -DBREAKPTS -D_BIG_ENDIAN= -DSOLARIS -DARCH='"sparcv9"' -Dsparcv9 -DNDEBUG -DTRIMMED -DRELEASE='"1.9.0-internal"' -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include/solaris -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/javavm/export -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/javavm/export -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/native/common -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/common -m64 -D__solaris__ -xc99=%none -xCC -errshort=tags -Xa -v -mt -W0,-noglobal -KPIC -xstrconst -xregs=no%appl -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/gensrc_headers -errtags -errwarn=%all -g -xs -xO2 -Wc,-Qrm-s -Wc,-Qiselect-T0 -DTHIS_FILE='"SolarisVirtualMachine.c"' -c -xMMD -xMF /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.d.tmp -o /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o /tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c "/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c", line 1: empty character constant "/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c", line 1: syntax error before or at: '' gmake[2]: *** [/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o] Error 1 This is blocking integration of patches that need to go in promptly. This needs to be addressed ASAP. -- Alejandro
Re: Build failures on solaris
Definitively a P1. This is also blocking this week hotspot snapshot: http://prt-web.us.oracle.com//archive/2014/05/2014-05-09-174238.amurillo.jdk9-hs-2014-05-09-jdk9-dev-control/logs/solaris_sparcv9_5.10-fastdebug.log.FAILED.log please fix ASAP Thanks Alejandro On 5/9/2014 12:16 PM, Eric McCorkle wrote: Bug created: https://bugs.openjdk.java.net/browse/JDK-8042859 This patch seems to be the culprit: changeset: 9908:7426549b1e3b tag: tip user:sla date:Fri May 09 12:06:13 2014 +0200 summary: 8039173: Propagate errors from Diagnostic Commands as exceptions in the attach framework On 05/09/14 14:08, Eric McCorkle wrote: I am currently seeing build failures on solaris, coming from within the JDK repo: /opt/jprt/products/P1/SS12u1/SS12u1/prod/bin/cc -DTRACING -DMACRO_MEMSYS_OPS -DBREAKPTS -D_BIG_ENDIAN= -DSOLARIS -DARCH='"sparcv9"' -Dsparcv9 -DNDEBUG -DTRIMMED -DRELEASE='"1.9.0-internal"' -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/include/solaris -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/javavm/export -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/javavm/export -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/share/native/common -I/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/common -m64 -D__solaris__ -xc99=%none -xCC -errshort=tags -Xa -v -mt -W0,-noglobal -KPIC -xstrconst -xregs=no%appl -I/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/gensrc_headers -errtags -errwarn=%all -g -xs -xO2 -Wc,-Qrm-s -Wc,-Qiselect-T0 -DTHIS_FILE='"SolarisVirtualMachine.c"' -c -xMMD -xMF /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.d.tmp -o /tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o /tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c "/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c", line 1: empty character constant "/tmp/jprt/P1/173806.emccorkl/s/jdk/src/solaris/native/sun/tools/attach/SolarisVirtualMachine.c", line 1: syntax error before or at: '' gmake[2]: *** [/tmp/jprt/P1/173806.emccorkl/s/build/solaris-sparcv9-normal-server-release/jdk/objs/libattach/SolarisVirtualMachine.o] Error 1 This is blocking integration of patches that need to go in promptly. This needs to be addressed ASAP. -- Alejandro
Re: Urgent: Broken build. Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java failing on all platforms
what's wrong with pushing them to jdk9/hs-rt? We did this a couple of weeks ago with Erik (Gahlin) changes, it might disrupt nightly, as we still do not have the JPRT changes in place, but that was the agreement we have for jdk9: tightly coupled changes should be pushed through the hotspot repos. had that been pushed this week there, it would be in jdk9/dev next Tuesday Alejandro On 4/25/2014 10:46 AM, Staffan Larsen wrote: Thanks Joe - fix has been pushed. (I will now retreat to a dark place and grumble over the impossibility of pushing coordinated changes). /Staffan On 25 apr 2014, at 18:43, Joe Darcy wrote: Approved! -Joe On 04/25/2014 09:36 AM, Staffan Larsen wrote: Here is an anti-delta for the broken push. I prepared it using “hg backout”. webrev: http://cr.openjdk.java.net/~sla/8041948/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8041948 If I can get this reviewed quickly I can push the fix soon (and I will spend the weekend in shame). Thanks, /Staffan On 25 apr 2014, at 18:24, Staffan Larsen wrote: It looks like a completely messed this up by not pushing the hotspot parts first and now I have broken the build in jdk9-dev. Should I push an anti-delta of the patch? I can prepare a review of it in a moment. /Staffan On 25 apr 2014, at 17:16, Staffan Larsen wrote: Thanks Keith! As far as I can tell there was no good reason for making the bug Confidential, but I can’t undo it. Sorry about that. /Staffan On 25 apr 2014, at 17:02, Keith McGuigan wrote: Hi Staffan - It looks good to me. Why is the bug marked "closed" though? On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen wrote: Still looking for a Review of this change. Thanks, /Staffan On 7 apr 2014, at 21:19, Staffan Larsen wrote: And the links: bug: https://bugs.openjdk.java.net/browse/JDK-8033104 webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/ Sorry about that, /Staffan On 7 apr 2014, at 20:08, Staffan Larsen wrote: The problem here is that the code for finding local VMs is not looking for the data in the correct place. When a JVM is started it will create the perf-data file in a user-specific directory inside /tmp (*). The code in the JDK (PerfDataFile.java) that lists all active JVMs looks for the user-specific directory inside java.io.tmpdir. If a user sets -Djava.io.tmpdir on the command line, the code in PerfDataFile will look in the wrong place. (*) It's a little bit more complex. /tmp is used on Linux and Solaris. On OS X and Windows, there are user-specific temp directories that should be used, and so the VM queries the OS for these paths. The solution would be for PerfDataFile to use the same locations as the VM creates them in. The simplest way to guarantee that the same directory is used is to ask the VM to provide the location. Thus I have introduced a new JVM_ function: JVM_GetTemporaryDirectory. (Since this change touches both hotspot and jdk repos I will submit the hotspot part first under a different bug id (provided that the review goes well)). The newly added test starts two VM with all possible combinations of setting and not setting java.io.tmpdir to verify that the mechanism is indeed not looking at that variable. I also removed an if-statement in BasicTests.java which would have found this issue a long time ago had it not been there. Thanks, /Staffan -- Keith McGuigan @kamggg kmcgui...@twitter.com -- Alejandro
