Re: RFR: JDK-8172912 JTReg concurrency value must be limited
I thought this was fixed a long time ago, meaning the limit was raised. -- Jon On 02/03/2017 04:59 AM, Magnus Ihse Bursie wrote: There is a limitation in jtreg that causes it to fail if called with -concurrency:X where X is > 50. This can happen on a multi-core machine, were we set the JOBS value to e.g. 64. Bug: https://bugs.openjdk.java.net/browse/JDK-8172912 Patch inline: diff --git a/test/Makefile b/test/Makefile --- a/test/Makefile +++ b/test/Makefile @@ -60,7 +60,12 @@ -include $(TOPDIR)/closed/test/Makefile ifeq ($(TEST_JOBS), 0) - JDK_TEST_JOBS=$(JOBS) + ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1) +# JTReg cannot handle more than 50 in concurrency +JDK_TEST_JOBS=50 + else +JDK_TEST_JOBS=$(JOBS) + endif else JDK_TEST_JOBS=$(TEST_JOBS) endif /Magnus
Re: RFR: JDK-8059000 hgforest: pass options to serve command
Looks good! Mike On 2017-02-03 02:21, Magnus Ihse Bursie wrote: Pass options to serve command in hgforest.sh. Mostly necessary for --port option. Original patch was written by Michael Duigou, but it has bit-rotted since created so I had to adapt it to the current hgforest.sh. Bug: https://bugs.openjdk.java.net/browse/JDK-8059000 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8059000-pass-options-to-hgforest-serve/webrev.01 /Magnus
Re: RFR: JDK-8172912 JTReg concurrency value must be limited
Does your jtreg have the latest javatest? https://bugs.openjdk.java.net/browse/CODETOOLS-7183756 On Fri, Feb 3, 2017 at 4:59 AM, Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: > There is a limitation in jtreg that causes it to fail if called with > -concurrency:X where X is > 50. This can happen on a multi-core machine, > were we set the JOBS value to e.g. 64. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8172912 > > Patch inline: > diff --git a/test/Makefile b/test/Makefile > --- a/test/Makefile > +++ b/test/Makefile > @@ -60,7 +60,12 @@ > -include $(TOPDIR)/closed/test/Makefile > > ifeq ($(TEST_JOBS), 0) > - JDK_TEST_JOBS=$(JOBS) > + ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1) > +# JTReg cannot handle more than 50 in concurrency > +JDK_TEST_JOBS=50 > + else > +JDK_TEST_JOBS=$(JOBS) > + endif > else >JDK_TEST_JOBS=$(TEST_JOBS) > endif > > /Magnus > >
RFR(S): JDK-8173894: jib reports version "" in jdk10
Hi,
Please review and sponsor this small patch to jib profiles.
It makes sure version parsing handles 10.
Thanks,
Stefan
- - -
# HG changeset patch
# User ssarne
# Date 1486133045 -3600
# Fri Feb 03 15:44:05 2017 +0100
# Node ID b0a2d45408861586b361cf19d6a3522f05d27bd9
# Parent d98052f2d47963a36e93f699a1cb85dc69e695be
JDK-8173894: jib reports version "" in jdk10
Summary: Update getVersion function, missing \ in regexp when stripping
trailing zeros.
Reviewed-by:
Contributed-by: stefan.sa...@oracle.com
diff -r d98052f2d479 -r b0a2d4540886 common/conf/jib-profiles.js
--- a/common/conf/jib-profiles.js Tue Jan 31 21:06:43 2017 -0500
+++ b/common/conf/jib-profiles.js Fri Feb 03 15:44:05 2017 +0100
@@ -1067,7 +1067,7 @@
+ "." + (minor != null ? minor :
version_numbers.get("DEFAULT_VERSION_MINOR"))
+ "." + (security != null ? security :
version_numbers.get("DEFAULT_VERSION_SECURITY"))
+ "." + (patch != null ? patch :
version_numbers.get("DEFAULT_VERSION_PATCH"));
-while (version.match(".*\.0$")) {
+while (version.match(".*\\.0$")) {
version = version.substring(0, version.length - 2);
}
return version;
Re: RFR(S): JDK-8173894: jib reports version "" in jdk10
Looks good, will push.
/Erik
On 2017-02-03 15:50, Stefan Sarne wrote:
Hi,
Please review and sponsor this small patch to jib profiles.
It makes sure version parsing handles 10.
Thanks,
Stefan
- - -
# HG changeset patch
# User ssarne
# Date 1486133045 -3600
# Fri Feb 03 15:44:05 2017 +0100
# Node ID b0a2d45408861586b361cf19d6a3522f05d27bd9
# Parent d98052f2d47963a36e93f699a1cb85dc69e695be
JDK-8173894: jib reports version "" in jdk10
Summary: Update getVersion function, missing \ in regexp when
stripping trailing zeros.
Reviewed-by:
Contributed-by: stefan.sa...@oracle.com
diff -r d98052f2d479 -r b0a2d4540886 common/conf/jib-profiles.js
--- a/common/conf/jib-profiles.js Tue Jan 31 21:06:43 2017 -0500
+++ b/common/conf/jib-profiles.js Fri Feb 03 15:44:05 2017 +0100
@@ -1067,7 +1067,7 @@
+ "." + (minor != null ? minor :
version_numbers.get("DEFAULT_VERSION_MINOR"))
+ "." + (security != null ? security :
version_numbers.get("DEFAULT_VERSION_SECURITY"))
+ "." + (patch != null ? patch :
version_numbers.get("DEFAULT_VERSION_PATCH"));
-while (version.match(".*\.0$")) {
+while (version.match(".*\\.0$")) {
version = version.substring(0, version.length - 2);
}
return version;
Re: RFR: JDK-8004842 Unify values of boolean make variables set in configure to true/false
Looks good. /Erik On 2017-02-03 14:14, Magnus Ihse Bursie wrote: Currently there are three different ways of expressing a boolean value in spec.gmk, true/false, yes/no and 1/0. We should only have one and since true/false is the most common one, we should stick to that. Since this bug was opened, most cases has been fixed but two remains: USE_PRECOMPILED_HEADER=1 ENABLE_INTREE_EC=yes Bug: https://bugs.openjdk.java.net/browse/JDK-8004842 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8004842-standardize-booleans-in-build/webrev.01 /Magnus
Re: RFR: JDK-8172912 JTReg concurrency value must be limited
Looks ok. /Erik On 2017-02-03 13:59, Magnus Ihse Bursie wrote: There is a limitation in jtreg that causes it to fail if called with -concurrency:X where X is > 50. This can happen on a multi-core machine, were we set the JOBS value to e.g. 64. Bug: https://bugs.openjdk.java.net/browse/JDK-8172912 Patch inline: diff --git a/test/Makefile b/test/Makefile --- a/test/Makefile +++ b/test/Makefile @@ -60,7 +60,12 @@ -include $(TOPDIR)/closed/test/Makefile ifeq ($(TEST_JOBS), 0) - JDK_TEST_JOBS=$(JOBS) + ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1) +# JTReg cannot handle more than 50 in concurrency +JDK_TEST_JOBS=50 + else +JDK_TEST_JOBS=$(JOBS) + endif else JDK_TEST_JOBS=$(TEST_JOBS) endif /Magnus
RFR: JDK-8004842 Unify values of boolean make variables set in configure to true/false
Currently there are three different ways of expressing a boolean value in spec.gmk, true/false, yes/no and 1/0. We should only have one and since true/false is the most common one, we should stick to that. Since this bug was opened, most cases has been fixed but two remains: USE_PRECOMPILED_HEADER=1 ENABLE_INTREE_EC=yes Bug: https://bugs.openjdk.java.net/browse/JDK-8004842 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8004842-standardize-booleans-in-build/webrev.01 /Magnus
RFR: JDK-8172912 JTReg concurrency value must be limited
There is a limitation in jtreg that causes it to fail if called with -concurrency:X where X is > 50. This can happen on a multi-core machine, were we set the JOBS value to e.g. 64. Bug: https://bugs.openjdk.java.net/browse/JDK-8172912 Patch inline: diff --git a/test/Makefile b/test/Makefile --- a/test/Makefile +++ b/test/Makefile @@ -60,7 +60,12 @@ -include $(TOPDIR)/closed/test/Makefile ifeq ($(TEST_JOBS), 0) - JDK_TEST_JOBS=$(JOBS) + ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1) +# JTReg cannot handle more than 50 in concurrency +JDK_TEST_JOBS=50 + else +JDK_TEST_JOBS=$(JOBS) + endif else JDK_TEST_JOBS=$(TEST_JOBS) endif /Magnus
Re: RFR: JDK-8059000 hgforest: pass options to serve command
Looks good. /Erik On 2017-02-03 13:21, Magnus Ihse Bursie wrote: Pass options to serve command in hgforest.sh. Mostly necessary for --port option. Original patch was written by Michael Duigou, but it has bit-rotted since created so I had to adapt it to the current hgforest.sh. Bug: https://bugs.openjdk.java.net/browse/JDK-8059000 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8059000-pass-options-to-hgforest-serve/webrev.01 /Magnus
Re: RFR [XXS] : cleanup macosx jspawnhelper build settings
Fine with me. /Erik On 2017-02-03 12:08, Baesken, Matthias wrote: Hi Erik, thanks for your ideas and comments. While your change is ok and can certainly be pushed on its own, there is so much more needing cleanup here. I would like to do the other suggested cleanups in a separate change. Is this fine, can the original change be pushed ? Best regards, Matthias -Original Message- From: Erik Joelsson [mailto:erik.joels...@oracle.com] Sent: Donnerstag, 2. Februar 2017 17:45 To: Baesken, Matthias ; build- d...@openjdk.java.net Cc: Simonis, Volker Subject: Re: RFR [XXS] : cleanup macosx jspawnhelper build settings Hello Matthias, While your change is ok and can certainly be pushed on its own, there is so much more needing cleanup here. If you don't mind, here is what I think needs doing: * Inline BUILD_JSPAWNHELPER_SRC, JSPAWNHELPER_CFLAGS, BUILD_JSPAWNHELPER_DST_DIR and LINK_JSPAWNHELPER_OBJECTS. Since these variables aren't conditionally changed anywhere, there is really no need for the indirection. * The whole business of "BUILD_JSPAWNHELPER_LDFLAGS += $(COMPILER_TARGET_BITS_FLAG)64" is confusing to me. Don't we trust the compiler for a 64 bit target to produce a 64 bit binary given the standard CFLAGS_JDKEXE and LDFLAGS_JDKLIB? I suspect this is just very old and confused code * The src dir only has the one src file, no need to explicitly list it for include. * The adding of childproc.o to LIBS can be accomplished using the parameter EXTRA_OBJECT_FILES. By using that you automatically get the dependency declaration so you can remove the line "$(BUILD_JSPAWNHELPER): $(LINK_JSPAWNHELPER_OBJECTS)" * The ifeq ($(BUILD_JSPAWNHELPER), 1) is also annoying, just move the single conditional into it's place. Thanks /Erik On 2017-02-02 17:25, Baesken, Matthias wrote: Hello, could I please have a review for the following small change that cleans up the special macosx BUILD_JSPAWNHELPER_DST_DIR setting that is not really needed any more after CR 8066474: "Remove the lib/ directory from Linux and Solaris images" changed the default setting . Bug : https://bugs.openjdk.java.net/browse/JDK-8173834 webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8173834.0/ Thanks, Matthias
RFR: JDK-8059000 hgforest: pass options to serve command
Pass options to serve command in hgforest.sh. Mostly necessary for --port option. Original patch was written by Michael Duigou, but it has bit-rotted since created so I had to adapt it to the current hgforest.sh. Bug: https://bugs.openjdk.java.net/browse/JDK-8059000 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8059000-pass-options-to-hgforest-serve/webrev.01 /Magnus
RE: RFR [XXS] : cleanup macosx jspawnhelper build settings
Hi Erik, thanks for your ideas and comments. > While your change is ok and can certainly be pushed on its own, there is > so much more needing cleanup here. I would like to do the other suggested cleanups in a separate change. Is this fine, can the original change be pushed ? Best regards, Matthias > -Original Message- > From: Erik Joelsson [mailto:erik.joels...@oracle.com] > Sent: Donnerstag, 2. Februar 2017 17:45 > To: Baesken, Matthias ; build- > d...@openjdk.java.net > Cc: Simonis, Volker > Subject: Re: RFR [XXS] : cleanup macosx jspawnhelper build settings > > Hello Matthias, > > While your change is ok and can certainly be pushed on its own, there is > so much more needing cleanup here. If you don't mind, here is what I > think needs doing: > > * Inline BUILD_JSPAWNHELPER_SRC, JSPAWNHELPER_CFLAGS, > BUILD_JSPAWNHELPER_DST_DIR and LINK_JSPAWNHELPER_OBJECTS. Since > these > variables aren't conditionally changed anywhere, there is really no need > for the indirection. > > * The whole business of "BUILD_JSPAWNHELPER_LDFLAGS += > $(COMPILER_TARGET_BITS_FLAG)64" is confusing to me. Don't we trust the > compiler for a 64 bit target to produce a 64 bit binary given the > standard CFLAGS_JDKEXE and LDFLAGS_JDKLIB? I suspect this is just very > old and confused code > > * The src dir only has the one src file, no need to explicitly list it > for include. > > * The adding of childproc.o to LIBS can be accomplished using the > parameter EXTRA_OBJECT_FILES. By using that you automatically get the > dependency declaration so you can remove the line > "$(BUILD_JSPAWNHELPER): $(LINK_JSPAWNHELPER_OBJECTS)" > > * The ifeq ($(BUILD_JSPAWNHELPER), 1) is also annoying, just move the > single conditional into it's place. > > Thanks > /Erik > > On 2017-02-02 17:25, Baesken, Matthias wrote: > > Hello, > > could I please have a review for the following small change that > > cleans > up the special macosx BUILD_JSPAWNHELPER_DST_DIR setting that is not > really > > needed any more after CR 8066474: "Remove the lib/ directory from > Linux and Solaris images" changed the default setting . > > > > > > Bug : > > > > https://bugs.openjdk.java.net/browse/JDK-8173834 > > > > webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8173834.0/ > > > > > > Thanks, Matthias
Re: Review Request: JDK-8173858: Rename libmanagement_rmi to libmanagement_agent
+1 libmanagement_agent is indeed a better name :-) best regards, -- daniel On 02/02/17 22:27, Mandy Chung wrote: libmanagement_agent should be the proper library name for jdk.management.agent. It was an oversight with the current name. http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173858/webrev.00/ This patch also takes out the qualified exports of java.base/jdk.internal.vm to java.management which is not needed. Mandy
Re: Review Request: JDK-8173858: Rename libmanagement_rmi to libmanagement_agent
Looks good. /Erik On 2017-02-02 23:27, Mandy Chung wrote: libmanagement_agent should be the proper library name for jdk.management.agent. It was an oversight with the current name. http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173858/webrev.00/ This patch also takes out the qualified exports of java.base/jdk.internal.vm to java.management which is not needed. Mandy
