Re: [8011944] Sort fails with ArrayIndexOutOfBoundsException
On 01/23/2014 11:55 PM, Miroslaw Niemiec wrote: Fix for 7: http://cr.openjdk.java.net/~miroslawzn/801144/webrev.01/ The new test lacks a license header. -- Florian Weimer / Red Hat Product Security Team
Re: JDK8 RFR JDK-8029237 Update copyright year to match last edit in jdk8 jaxws repository for 2013
Hi Chris, thanks for reviewing - I generated the patches today again so now it should be ok: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012-v02.patch http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013-v02.patch Could you push it for me? I have no rights for jdk8. I suppose the commit dates (must be in the past) would be imported correctly: copyrights-2012-v02.patch: -d 2012-12-30 copyrights-2013-v02.patch: -d 2013-12-30 Thanks Miran On 23/01/14 10:07, Chris Hegarty wrote: On 23 Jan 2014, at 08:53, Martin Grebac martin.gre...@oracle.com wrote: Hi Miran, did you get any response on this yet? MartiNG On 17/01/14 10:57, Miroslav Kos wrote: Hi Steve and Alan, I just reminding this issue - I have no response from anybody yet and it would really simplify our integration if I could handle copyright years inconsistencies this way - I see this issue in many jdk branches and these each-branch-different changes make each integration much more pain. Thanks for any idea on this. Miran On 10/01/14 18:05, Miroslav Kos wrote: Hi, this is about fixing copyright years for jdk8 (!); the incorrect years found for both 2012 and 2013, so not to confuse script jdk8/make/scripts/update_copyright_year.sh it is necessary to use option -d date for commit. I tested that and it seems to work perfect. I used following: [2012 modifications] hg commit -u mkos -l ../2012-msg.txt -d 2012-12-30 [2013 modifications] hg commit -u mkos -l ../2013-msg.txt -d 2013-12-30 When commits performed with past date, update_copyright_year.sh script finds those in proper years. Since I am not familiar with using webrev for pushing changes, I exported both revisions into separate patches: 2012: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012.patch I skimmed the patch and it looks ok to me. Trivially, it contains a 2013 update (which it should probably be in the 2013 patch) diff -r 91f5c542ccad -r d6aec26c9771 make/Makefile --- a/make/Makefile Fri Jan 03 11:54:55 2014 -0800 +++ b/make/Makefile Sun Dec 30 00:00:00 2012 +0100 @@ -1,5 +1,5 @@ # -# Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # 2013: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013.patch Looks ok to me. -Chris. all changes webrev: http://cr.openjdk.java.net/~mkos/8029237/webrev-all.00/ (upload still in progress, I hope it doesn't take more than 30 mins) bug: https://bugs.openjdk.java.net/browse/JDK-8029237 Regards Miran -- Martin Grebac, SW Engineering Manager Oracle Czech, Prague
Re: Question about the bug https://bugs.openjdk.java.net/browse/JDK-8031179
Hi Stuart, Please review the webrev http://cr.openjdk.java.net/~ewang/JDK-8031179/webrev.00/, if you are OK with the changes, could you please be my sponsor? Thanks, Eric On 2014/1/24 15:14, Eric Wang wrote: Hi Stuart, Thanks for the suggestion! sorry for reply this mail late as i was busy on other tasks The webrev has been in the internal review process. Based on the suggestion, here is a summary of changes: 1. Add othervm options to tests: java/rmi/Naming/DefaultRegistryPort.java java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java java/rmi/Naming/LookupNameWithColon.java java/rmi/server/UnicastRemoteObject/exportObject/GcDuringExport.java sun/rmi/rmic/RMIGenerator/RmicDefault.java java/rmi/MarshalledObject/compare/Compare.java java/rmi/MarshalledObject/compare/HashCode.java 2. Remove java/rmi and sun/rmi from othervm.dirs of TEST.ROOT 3. Filed a another bug https://bugs.openjdk.java.net/browse/JDK-8032629 for exclusiveAccess.dirs Thanks, Eric On 2014/1/18 8:45, Stuart Marks wrote: Hi Eric, Thanks for doing the analysis of the tests that need /othervm. The list of tests that don't need /othervm looks good. One subtlety is this one: java/rmi/activation/ActivationGroupDesc/checkDefaultGroupName/CheckDefaultGroupName.java Most activation tests do need /othervm, but this one doesn't, since all it does is create an ActivationGroupDesc instance, which has no side effects. This is unusual for the activation APIs, since most of them are quite intertwined with the rest of the activation subsystem. So, for this test, the lack of /othervm warrants a comment explaining why /othervm is unnecessary. Regarding merging of the tests java/rmi/MarshalledObject/compare/Compare.java and HashCode.java, this is fairly subtle and not obviously wrong, but it's not obviously right either. Note that different jtreg tests -- even in agentvm or samevm mode -- load the test class in a fresh classloader, which means that the statics are reinitialized. This provides some degree of isolation of the tests even when they're reusing the same JVM. By contrast, calling the two different methods from within the same test exposes the second one to initializations left from the first one. In addition (though I'm not too concerned about this) if the first test fails the second won't be run at all. Merging these tests is kind of out of scope for this particular bug, and since I wasn't fully able to convince myself that the merged tests have the same semantics as the unmerged tests, I'd prefer not to see the merging of these tests as part of this changeset. (Some cleanup of these two tests is probably warranted eventually. I'd take a different approach of merging and refactoring the sources but keeping them as separate test runs, using two @run tags. But we should work on that separately. Meanwhile, the overhead of having these as separate tests is minimal, especially if they're not run in /othervm mode.) I don't think the removal of java/rmi/Naming from exclusiveAccess.dirs is safe at this point. The DefaultRegistryPort.java test uses the default registry port, not a unique one. Conceptually it would need to be converted to use the TestLibrary unique port stuff, but the test is actually about using the default port. So, the solution here isn't obvious. In addition, the java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java test also uses the default registry port. It too will need to converted before java/rmi/Naming is removed from exclusiveAccess.dirs. The java/rmi/Naming/LookupIPv6.java test was converted to use the TestLibrary unique port technique, but only partially. The registry is created on the unique port, but the Naming.lookup() call on the last line of the test doesn't provide a port number, so it does the lookup on the default port instead. This will cause the test to fail in almost all cases. I have to ask, did you run this test with your modifications? (Well, the test would pass if IPv6 is not enabled on the machine running the test, but it only passes because the part of the test that creates and uses the registry is bypassed entirely if IPv6 is not enabled. If you're modifying code, you need to take responsibility for ensuring that the code being modified is actually being run and is doing what you expect.) LookupIPv6.java also needs to have these lines added to the test tags: * @bug 4402708 + * @library ../testlibrary + * @build TestLibrary * @run main/othervm -Djava.net.preferIPv6Addresses=true LookupIPv6 (Their absence will cause the test also to fail in a clean build, but the test will find a TestLibrary class if one had been compiled by a previous test that had required it.) Maybe we should separate the othervm changes (removal of the rmi dirs from othervm.dirs, and addition of /othervm) from the exclusiveAccess.dirs changes. A separate bug would be filed for exclusiveAccess.dirs. I know this means the bug count won't
Re: JDK8 RFR JDK-8029237 Update copyright year to match last edit in jdk8 jaxws repository for 2013
Miran, This bug has not been approved for JDK 8, in fact it was considered and rejected. The fix should be pushed to jdk9/dev, if applicable. If the patches apply cleanly I can push it for you. Do you want it in JDK 9? -Chris. On 24/01/14 09:41, Miroslav Kos wrote: Hi Chris, thanks for reviewing - I generated the patches today again so now it should be ok: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012-v02.patch http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013-v02.patch Could you push it for me? I have no rights for jdk8. I suppose the commit dates (must be in the past) would be imported correctly: copyrights-2012-v02.patch: -d 2012-12-30 copyrights-2013-v02.patch: -d 2013-12-30 Thanks Miran On 23/01/14 10:07, Chris Hegarty wrote: On 23 Jan 2014, at 08:53, Martin Grebac martin.gre...@oracle.com wrote: Hi Miran, did you get any response on this yet? MartiNG On 17/01/14 10:57, Miroslav Kos wrote: Hi Steve and Alan, I just reminding this issue - I have no response from anybody yet and it would really simplify our integration if I could handle copyright years inconsistencies this way - I see this issue in many jdk branches and these each-branch-different changes make each integration much more pain. Thanks for any idea on this. Miran On 10/01/14 18:05, Miroslav Kos wrote: Hi, this is about fixing copyright years for jdk8 (!); the incorrect years found for both 2012 and 2013, so not to confuse script jdk8/make/scripts/update_copyright_year.sh it is necessary to use option -d date for commit. I tested that and it seems to work perfect. I used following: [2012 modifications] hg commit -u mkos -l ../2012-msg.txt -d 2012-12-30 [2013 modifications] hg commit -u mkos -l ../2013-msg.txt -d 2013-12-30 When commits performed with past date, update_copyright_year.sh script finds those in proper years. Since I am not familiar with using webrev for pushing changes, I exported both revisions into separate patches: 2012: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012.patch I skimmed the patch and it looks ok to me. Trivially, it contains a 2013 update (which it should probably be in the 2013 patch) diff -r 91f5c542ccad -r d6aec26c9771 make/Makefile --- a/make/MakefileFri Jan 03 11:54:55 2014 -0800 +++ b/make/MakefileSun Dec 30 00:00:00 2012 +0100 @@ -1,5 +1,5 @@ # -# Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # 2013: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013.patch Looks ok to me. -Chris. all changes webrev: http://cr.openjdk.java.net/~mkos/8029237/webrev-all.00/ (upload still in progress, I hope it doesn't take more than 30 mins) bug: https://bugs.openjdk.java.net/browse/JDK-8029237 Regards Miran -- Martin Grebac, SW Engineering Manager Oracle Czech, Prague
Re: JDK8 RFR JDK-8029237 Update copyright year to match last edit in jdk8 jaxws repository for 2013
Hi Chris, I missed that, sorry. Yes please push it to jdk9, it will do the job there too... Thanks Miran On 24/01/14 11:12, Chris Hegarty wrote: Miran, This bug has not been approved for JDK 8, in fact it was considered and rejected. The fix should be pushed to jdk9/dev, if applicable. If the patches apply cleanly I can push it for you. Do you want it in JDK 9? -Chris. On 24/01/14 09:41, Miroslav Kos wrote: Hi Chris, thanks for reviewing - I generated the patches today again so now it should be ok: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012-v02.patch http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013-v02.patch Could you push it for me? I have no rights for jdk8. I suppose the commit dates (must be in the past) would be imported correctly: copyrights-2012-v02.patch: -d 2012-12-30 copyrights-2013-v02.patch: -d 2013-12-30 Thanks Miran On 23/01/14 10:07, Chris Hegarty wrote: On 23 Jan 2014, at 08:53, Martin Grebac martin.gre...@oracle.com wrote: Hi Miran, did you get any response on this yet? MartiNG On 17/01/14 10:57, Miroslav Kos wrote: Hi Steve and Alan, I just reminding this issue - I have no response from anybody yet and it would really simplify our integration if I could handle copyright years inconsistencies this way - I see this issue in many jdk branches and these each-branch-different changes make each integration much more pain. Thanks for any idea on this. Miran On 10/01/14 18:05, Miroslav Kos wrote: Hi, this is about fixing copyright years for jdk8 (!); the incorrect years found for both 2012 and 2013, so not to confuse script jdk8/make/scripts/update_copyright_year.sh it is necessary to use option -d date for commit. I tested that and it seems to work perfect. I used following: [2012 modifications] hg commit -u mkos -l ../2012-msg.txt -d 2012-12-30 [2013 modifications] hg commit -u mkos -l ../2013-msg.txt -d 2013-12-30 When commits performed with past date, update_copyright_year.sh script finds those in proper years. Since I am not familiar with using webrev for pushing changes, I exported both revisions into separate patches: 2012: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2012.patch I skimmed the patch and it looks ok to me. Trivially, it contains a 2013 update (which it should probably be in the 2013 patch) diff -r 91f5c542ccad -r d6aec26c9771 make/Makefile --- a/make/MakefileFri Jan 03 11:54:55 2014 -0800 +++ b/make/MakefileSun Dec 30 00:00:00 2012 +0100 @@ -1,5 +1,5 @@ # -# Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # 2013: http://cr.openjdk.java.net/~mkos/8029237/copyrights-2013.patch Looks ok to me. -Chris. all changes webrev: http://cr.openjdk.java.net/~mkos/8029237/webrev-all.00/ (upload still in progress, I hope it doesn't take more than 30 mins) bug: https://bugs.openjdk.java.net/browse/JDK-8029237 Regards Miran -- Martin Grebac, SW Engineering Manager Oracle Czech, Prague
8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
I need a reviewer to fix an issue with the changes in JDK 8 to support
statically linked JNI libraries. The issue arises with tests that have
both JNI_OnLoad and JNI_OnLoad_libname functions defined, and code
attempts to load the library more than once. In that scenario then both
functions are called.
Staffan Larsen did the hard work in JDK-8031968 where he observed that
dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should
be using dlopen(NULL, RTLD_FIRST) to get the handle of the main program.
Staffan has fixed this in the hotspot repository for JVM TI agent
libraries, we need to do the same in the libjava code used by
ClassLoader's native methods.
Note that the include of string.h is not directly part of the issue
here, instead it's just a drive-by fix to the warning related to the
strcat usages.
-Alan
diff --git a/src/solaris/native/common/jni_util_md.c
b/src/solaris/native/common/jni_util_md.c
--- a/src/solaris/native/common/jni_util_md.c
+++ b/src/solaris/native/common/jni_util_md.c
@@ -23,6 +23,8 @@
* questions.
*/
+#include string.h
+
#include jni.h
#include jni_util.h
#include dlfcn.h
@@ -40,7 +42,11 @@
if (procHandle != NULL) {
return procHandle;
}
-procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#ifdef __APPLE__
+procHandle = (void*)dlopen(NULL, RTLD_FIRST);
+#else
+procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#endif
return procHandle;
}
Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
Looks good! :-)
Thanks,
/Staffan
On 24 jan 2014, at 12:00, Alan Bateman alan.bate...@oracle.com wrote:
I need a reviewer to fix an issue with the changes in JDK 8 to support
statically linked JNI libraries. The issue arises with tests that have both
JNI_OnLoad and JNI_OnLoad_libname functions defined, and code attempts to
load the library more than once. In that scenario then both functions are
called.
Staffan Larsen did the hard work in JDK-8031968 where he observed that
dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should be
using dlopen(NULL, RTLD_FIRST) to get the handle of the main program. Staffan
has fixed this in the hotspot repository for JVM TI agent libraries, we need
to do the same in the libjava code used by ClassLoader's native methods.
Note that the include of string.h is not directly part of the issue here,
instead it's just a drive-by fix to the warning related to the strcat usages.
-Alan
diff --git a/src/solaris/native/common/jni_util_md.c
b/src/solaris/native/common/jni_util_md.c
--- a/src/solaris/native/common/jni_util_md.c
+++ b/src/solaris/native/common/jni_util_md.c
@@ -23,6 +23,8 @@
* questions.
*/
+#include string.h
+
#include jni.h
#include jni_util.h
#include dlfcn.h
@@ -40,7 +42,11 @@
if (procHandle != NULL) {
return procHandle;
}
-procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#ifdef __APPLE__
+procHandle = (void*)dlopen(NULL, RTLD_FIRST);
+#else
+procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#endif
return procHandle;
}
Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
The change looks good to me Alan.
-Chris.
On 24/01/14 11:00, Alan Bateman wrote:
I need a reviewer to fix an issue with the changes in JDK 8 to support
statically linked JNI libraries. The issue arises with tests that have
both JNI_OnLoad and JNI_OnLoad_libname functions defined, and code
attempts to load the library more than once. In that scenario then both
functions are called.
Staffan Larsen did the hard work in JDK-8031968 where he observed that
dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should
be using dlopen(NULL, RTLD_FIRST) to get the handle of the main program.
Staffan has fixed this in the hotspot repository for JVM TI agent
libraries, we need to do the same in the libjava code used by
ClassLoader's native methods.
Note that the include of string.h is not directly part of the issue
here, instead it's just a drive-by fix to the warning related to the
strcat usages.
-Alan
diff --git a/src/solaris/native/common/jni_util_md.c
b/src/solaris/native/common/jni_util_md.c
--- a/src/solaris/native/common/jni_util_md.c
+++ b/src/solaris/native/common/jni_util_md.c
@@ -23,6 +23,8 @@
* questions.
*/
+#include string.h
+
#include jni.h
#include jni_util.h
#include dlfcn.h
@@ -40,7 +42,11 @@
if (procHandle != NULL) {
return procHandle;
}
-procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#ifdef __APPLE__
+procHandle = (void*)dlopen(NULL, RTLD_FIRST);
+#else
+procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#endif
return procHandle;
}
Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
On Jan 24, 2014, at 12:00 PM, Alan Bateman alan.bate...@oracle.com wrote:
I need a reviewer to fix an issue with the changes in JDK 8 to support
statically linked JNI libraries. The issue arises with tests that have both
JNI_OnLoad and JNI_OnLoad_libname functions defined, and code attempts to
load the library more than once. In that scenario then both functions are
called.
Staffan Larsen did the hard work in JDK-8031968 where he observed that
dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should be
using dlopen(NULL, RTLD_FIRST) to get the handle of the main program. Staffan
has fixed this in the hotspot repository for JVM TI agent libraries, we need
to do the same in the libjava code used by ClassLoader's native methods.
IIUC from reading the docs:
https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/dlopen.3.html
...
discovered during the call to dlopen(). If neither RTLD_LAZY nor RTLD_NOW
is specified, the default is RTLD_LAZY.
...
RTLD_FIRST The retuned handle is tagged so that any dlsym() calls on the
handle will only search the image specified, and not
subsequent images. If path is NULL and the option RTLD_FIRST
is used, the handle returned will only search the main
executable.
So more precisely that is RTLD_LAZY | RTLD_FIRST ?
Paul.
Note that the include of string.h is not directly part of the issue here,
instead it's just a drive-by fix to the warning related to the strcat usages.
-Alan
diff --git a/src/solaris/native/common/jni_util_md.c
b/src/solaris/native/common/jni_util_md.c
--- a/src/solaris/native/common/jni_util_md.c
+++ b/src/solaris/native/common/jni_util_md.c
@@ -23,6 +23,8 @@
* questions.
*/
+#include string.h
+
#include jni.h
#include jni_util.h
#include dlfcn.h
@@ -40,7 +42,11 @@
if (procHandle != NULL) {
return procHandle;
}
-procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#ifdef __APPLE__
+procHandle = (void*)dlopen(NULL, RTLD_FIRST);
+#else
+procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#endif
return procHandle;
}
Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
On 24/01/2014 11:19, Paul Sandoz wrote: : So more precisely that is RTLD_LAZY | RTLD_FIRST ? Either will do, I went for RTLD_FIRST to be consistent with the hotspot change. -Alan
MutableCallSite.syncAll() (Re: RFR: Update code in java.lang to use new language features)
On 01/24/2014 06:45 PM, Paul Sandoz wrote:
I think the update to java.lang.invoke.MutableCallSite.java should be safe:
public static void syncAll(MutableCallSite[] sites) {
if (sites.length == 0) return;
STORE_BARRIER.lazySet(0);
-for (int i = 0; i sites.length; i++) {
-sites[i].getClass(); // trigger NPE on first null
+for (MutableCallSite site : sites) {
+site.getClass(); // trigger NPE on first null
}
// FIXME: NYI
}
However, that got me wondering about that STORE_BARRIER. IIUC it is
using a static AtomicInteger and a call to lazySet to in effect
emulate a StoreStore (release) barrier or the equivalent of
Unsafe.storeFence, so the AtomicInteger could be replaced with
Unsafe. (When there is an update to the JMM it might be possible to
update docs on syncAll.)
Thanks for reminding me about this.
The JMM Details outlined in Javadoc for MutableCallSite.syncAll() [1]
seem wishful thinking to me. Particularly the part when the notion of SO
totality somehow transforms into visibility for
non-synchronization-action reads. The point about synchronization-order
consistency is a moot, because it is only spec-ed for volatile actions.
The visibility guarantees for everything else are provided by HB, which
is not applicable here. That is, spec-wise, without the paired volatile
read of the same AtomicInteger observing the written value, we are not
guaranteed to see the updated MutableCallSite value.
Since JEP 188 tries to also rigorously spec fences, I'll ping Doug to
add this into JEP 188 tally.
-Aleksey.
[1]
http://docs.oracle.com/javase/7/docs/api/java/lang/invoke/MutableCallSite.html#syncAll(java.lang.invoke.MutableCallSite[])
Survey on sun.misc.Unsafe
Hi, We are gathering feedback on sun.misc.Unsafe usage. If you have ever used it please consider taking this survey and helping us out: https://www.surveymonkey.com/s/sun-misc-Unsafe -- We also plan to trawl stuff on repos. I have done a selective bit of that already, running with a little tool spitting out method usage stats. I would have included a link to that tool but it is dependent on Java 8. If anyone still wants to use it let me know and i can put that code on GitHub. Paul.
Re: RFR: Update code in java.lang to use new language features
On 01/24/2014 06:45 AM, Paul Sandoz wrote: Hi, Here are some patches that update code in java.lang to use newer language features. I will log a bug and fold into one patch once reviewed. Locally run java.lang tests all pass, but i will also kick off a JPRT test. Paul. -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-diamonds/ -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-foreach/ I've looked over diamond and for-each and they both look good; approved to go back :-) -Joe
RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker
Re: RFR: Update code in java.lang to use new language features
On 24/01/2014 14:45, Paul Sandoz wrote: -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-StringBuilder/ -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-collapse-catches/ -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-boxing/ I looked through the StringBuilder, multi-catch and the improvement to ConditionalSpecialCasing's initializer. Looks okay to me. -Alan.
Re: RFR: Update code in java.lang to use new language features
Hi Joe, Alan, Many thanks! Before folding/committing i will kick off a JPRT job (if i can get it to work on 9!) -- BTW if one uses IntelliJ it is very easy to make such changes, review and tweak them. Select menu item Analyze - Run Inspection By Name... and type in migration. Hint hint for anyone else who wants to have a go on other packages :-) Paul. On Jan 24, 2014, at 6:02 PM, Alan Bateman alan.bate...@oracle.com wrote: On 24/01/2014 14:45, Paul Sandoz wrote: -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-StringBuilder/ -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-collapse-catches/ -- http://cr.openjdk.java.net/~psandoz/jdk9/java.lang-boxing/ I looked through the StringBuilder, multi-catch and the improvement to ConditionalSpecialCasing's initializer. Looks okay to me. -Alan.
Re: RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
Good change. I got this failure too. Thanks, Vladimir On 1/24/14 9:04 AM, Volker Simonis wrote: Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker
Re: 8032456: vm/jni/Miscellaneous/misc001/misc00101m1/misc00101m1.html failing on OS X
This patch looks fine.
For JDK 9, it might be worth considering using the common code via JVM
functions to replace getProcessHandle, JDK_FindJvmEntry,
JDK_InitJvmEntry for native symbol lookup (jni_util.h and jdk_util.h)
Mandy
On 1/24/2014 3:00 AM, Alan Bateman wrote:
I need a reviewer to fix an issue with the changes in JDK 8 to support
statically linked JNI libraries. The issue arises with tests that have
both JNI_OnLoad and JNI_OnLoad_libname functions defined, and code
attempts to load the library more than once. In that scenario then
both functions are called.
Staffan Larsen did the hard work in JDK-8031968 where he observed that
dlopen(NULL, RTLD_LAZY) behaves differently on OS X and that we should
be using dlopen(NULL, RTLD_FIRST) to get the handle of the main
program. Staffan has fixed this in the hotspot repository for JVM TI
agent libraries, we need to do the same in the libjava code used by
ClassLoader's native methods.
Note that the include of string.h is not directly part of the issue
here, instead it's just a drive-by fix to the warning related to the
strcat usages.
-Alan
diff --git a/src/solaris/native/common/jni_util_md.c
b/src/solaris/native/common/jni_util_md.c
--- a/src/solaris/native/common/jni_util_md.c
+++ b/src/solaris/native/common/jni_util_md.c
@@ -23,6 +23,8 @@
* questions.
*/
+#include string.h
+
#include jni.h
#include jni_util.h
#include dlfcn.h
@@ -40,7 +42,11 @@
if (procHandle != NULL) {
return procHandle;
}
-procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#ifdef __APPLE__
+procHandle = (void*)dlopen(NULL, RTLD_FIRST);
+#else
+procHandle = (void*)dlopen(NULL, RTLD_LAZY);
+#endif
return procHandle;
}
Re: RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
Hi Vladimir, I've just pushed this to jdk9/dev. I also received an email stating that this bug needs to be updated, please request to defer or upgrade to P1 and request for approval. I assume that's because I entered 8-pool, 9 as Fix versions. Now I actually don't know what to do. It seems strange to upgrade this to P1 because it is only a test bug. On the other hand it is needed in 8u because there's where we have the trouble. So should I upgrade it to P1 and request approval for jdk8u-dev (or directly jdk8u)? The actual problem occurs in jdk8u only, because jdk8u-dev doesn't yet has change 8032678 which bumps the HS minor number to 20. Can I push directly to jdk8u or does the change has to go trough jdk8u-dev? Thanks, Volker On Fri, Jan 24, 2014 at 6:13 PM, Vladimir Kozlov vladimir.koz...@oracle.com wrote: Good change. I got this failure too. Thanks, Vladimir On 1/24/14 9:04 AM, Volker Simonis wrote: Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker
Re: RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
And I just noticed from Alejandro's mail that there also exists hs24.60 for 7u60 now. So if somebody is interested in cleanly passing the jtreg tests on 7u60 this change will also has to be backported to 7u. On Fri, Jan 24, 2014 at 7:39 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi Vladimir, I've just pushed this to jdk9/dev. I also received an email stating that this bug needs to be updated, please request to defer or upgrade to P1 and request for approval. I assume that's because I entered 8-pool, 9 as Fix versions. Now I actually don't know what to do. It seems strange to upgrade this to P1 because it is only a test bug. On the other hand it is needed in 8u because there's where we have the trouble. So should I upgrade it to P1 and request approval for jdk8u-dev (or directly jdk8u)? The actual problem occurs in jdk8u only, because jdk8u-dev doesn't yet has change 8032678 which bumps the HS minor number to 20. Can I push directly to jdk8u or does the change has to go trough jdk8u-dev? Thanks, Volker On Fri, Jan 24, 2014 at 6:13 PM, Vladimir Kozlov vladimir.koz...@oracle.com wrote: Good change. I got this failure too. Thanks, Vladimir On 1/24/14 9:04 AM, Volker Simonis wrote: Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker
Re: RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
I've removed the incorrect-for-JBS multiple release values of 8-pool, 9 in the fixVersion field of the main bug record. -Joe On 1/24/2014 10:39 AM, Volker Simonis wrote: Hi Vladimir, I've just pushed this to jdk9/dev. I also received an email stating that this bug needs to be updated, please request to defer or upgrade to P1 and request for approval. I assume that's because I entered 8-pool, 9 as Fix versions. Now I actually don't know what to do. It seems strange to upgrade this to P1 because it is only a test bug. On the other hand it is needed in 8u because there's where we have the trouble. So should I upgrade it to P1 and request approval for jdk8u-dev (or directly jdk8u)? The actual problem occurs in jdk8u only, because jdk8u-dev doesn't yet has change 8032678 which bumps the HS minor number to 20. Can I push directly to jdk8u or does the change has to go trough jdk8u-dev? Thanks, Volker On Fri, Jan 24, 2014 at 6:13 PM, Vladimir Kozlov vladimir.koz...@oracle.com wrote: Good change. I got this failure too. Thanks, Vladimir On 1/24/14 9:04 AM, Volker Simonis wrote: Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker
Re: RFR(XS): 8032678: [TESTBUG] sun/misc/Version/Version.java doesn't understand two-digit HotSpot minor version numbers
I updated Fix versions to 8u20. No need to upgrade since you are not pushing to jdk8. You need to send request for approval to push into 8u20 to jdk8u-...@openjdk.java.net I attached example. After approval, you can push to: http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk Regards, Vladimir On 1/24/14 10:39 AM, Volker Simonis wrote: Hi Vladimir, I've just pushed this to jdk9/dev. I also received an email stating that this bug needs to be updated, please request to defer or upgrade to P1 and request for approval. I assume that's because I entered 8-pool, 9 as Fix versions. Now I actually don't know what to do. It seems strange to upgrade this to P1 because it is only a test bug. On the other hand it is needed in 8u because there's where we have the trouble. So should I upgrade it to P1 and request approval for jdk8u-dev (or directly jdk8u)? The actual problem occurs in jdk8u only, because jdk8u-dev doesn't yet has change 8032678 which bumps the HS minor number to 20. Can I push directly to jdk8u or does the change has to go trough jdk8u-dev? Thanks, Volker On Fri, Jan 24, 2014 at 6:13 PM, Vladimir Kozlov vladimir.koz...@oracle.com wrote: Good change. I got this failure too. Thanks, Vladimir On 1/24/14 9:04 AM, Volker Simonis wrote: Hi, could you please review the following trivial change: http://cr.openjdk.java.net/~simonis/webrevs/8032678/ which fixes the JDK test jdk/test/sun/misc/Version/Version.java to correctly handle two-digit HotSpot minor version numbers. This became necessary after change 8031552: Update the Hotspot version numbers in Hotspot for JDK 8U bumped the minor HS version to '20' in jdk8u. This webrev is for jdk9 but it will have to be backported to jdk8u because there’s where the current problem happens. Thanks, Volker ---BeginMessage--- Hi, This is a request to backport the following change: JDK-801 Issue: https://bugs.openjdk.java.net/browse/JDK-801 Changeset: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/3cbeb88b8735 Review : http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-January/013801.html The patch applies cleanly without any changes necessary. Cheers, -JB- ---End Message---
RFR (JAXP): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)
Hi, Please review a javadoc change to javax.xml.stream factories. This change makes it clear that the two args ServiceLoader#load method is used when the specified classLoader is not null. http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/ Thanks, Joe
Re: RFR (JAXP): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)
Hi, It looks fine to me Joe. best regards, -- daniel On 1/24/14 9:31 PM, huizhe wang wrote: Hi, Please review a javadoc change to javax.xml.stream factories. This change makes it clear that the two args ServiceLoader#load method is used when the specified classLoader is not null. http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/ Thanks, Joe
Re: RFR (JAXP): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)
+1 On Jan 24, 2014, at 3:31 PM, huizhe wang wrote: Hi, Please review a javadoc change to javax.xml.stream factories. This change makes it clear that the two args ServiceLoader#load method is used when the specified classLoader is not null. http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/ Thanks, Joe Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR (JAXP): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)
Thanks Lance, Daniel. On 1/24/2014 12:53 PM, Lance Andersen - Oracle wrote: +1 On Jan 24, 2014, at 3:31 PM, huizhe wang wrote: Hi, Please review a javadoc change to javax.xml.stream factories. This change makes it clear that the two args ServiceLoader#load method is used when the specified classLoader is not null. http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/ Thanks, Joe Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: Which optimizations does Hotspot apply?
Hi Rémi, With latest jdk8, it's not true anymore. (and most of the time the iterator object is not created anymore at least with jdk7+). Could you please explain it a little bit more? When is that optimization applied, e.g. what conditions are required for this optimization, since which version of JDK/Hotspot it is supported, where it is implemented in JDK? When I take look at a product I'm working on, I see a lot instances of ArrayList$Itr objects, which are created by for-each loops (we use JDK 7u51). Thanks in advance! Best regards, Andrej Golovnin
Re: Which optimizations does Hotspot apply?
This would be predicated on escape analysis, whose effectiveness is in turn driven by sufficient inlining whereby compiler can see that instance doesn't escape. Inlining, in turn, can be screwed up by (amongst other things) polymorphic call sites. So in the end, it's all quite brittle and sensitive :). Personally, in places where it matters, I try to allocate as little as possible explicitly - a lot of times much more effective than hoping compiler helps out and/or turning GC knobs. Sent from my phone On Jan 24, 2014 4:36 PM, Andrej Golovnin golov...@gmx.net wrote: Hi Rémi, With latest jdk8, it's not true anymore. (and most of the time the iterator object is not created anymore at least with jdk7+). Could you please explain it a little bit more? When is that optimization applied, e.g. what conditions are required for this optimization, since which version of JDK/Hotspot it is supported, where it is implemented in JDK? When I take look at a product I'm working on, I see a lot instances of ArrayList$Itr objects, which are created by for-each loops (we use JDK 7u51). Thanks in advance! Best regards, Andrej Golovnin
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 01/24/2014 02:53 AM, srikalyan chandrashekar wrote:
Hi David, yes thats right, only benefit i see is we can avoid
assignment to 'r' if pending is null.
Hi Kalyan,
Good to hear that test runs without failures so far.
Regarding assignment of 'r'. What I tried to accomplish with the change
was eliminate double reading of 'pending' field. I have a mental model
of local variable being a register and field being a memory location.
This may be important if the field is volatile, but for normal fields, I
guess the optimizer knows how to compile such code most optimally in
either case. The old (your) version is better from logical perspective,
since it guarantees that dereferencing the 'r', wherever it is possible,
will never throw NPE (dereferencing where 'r' is not assigned is not
possible because of definitive assignment rules). So I support going
back to your version...
Regards, Peter
--
Thanks
kalyan
On 1/23/14 4:33 PM, David Holmes wrote:
On 24/01/2014 6:10 AM, srikalyan wrote:
Hi Peter, i have modified your code from
r = pending;
if (r != null) {
..
TO
if (pending != null) {
r = pending;
This is because the r is used later in the code and must not be
assigned
pending unless it is not null(this was as is earlier).
If r is null, because pending is null then you perform the wait() and
then continue - back to the top of the loop. There is no bug in
Peter's code.
The new webrev is
posted here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev-V2/
. I ran a 1000 run and no failures so far, however i would like to
run a
couple more 1000 runs to assert the fix.
PS: The description section of JEP-122
(http://openjdk.java.net/jeps/122) says meta-data would be in native
memory(not heap).
The class_mirror is a Java object not meta-data.
David
--
Thanks
kalyan
Ph: (408)-585-8040
On 1/21/14, 2:31 PM, Peter Levart wrote:
On 01/21/2014 07:17 PM, srikalyan wrote:
Hi Peter/David, catching up after long weekend. Why would there be an
OOME in object heap due to class loading in perm gen space ?
The perm gen is not a problem her (JDK 8 does not have it and we see
OOME on JDK8 too). Each time a class is loaded, new java.lang.Class
object is allocated on heap.
Regards, Peter
Please correct if i am missing something here. Meanwhile i will give
the version of Reference Handler you both agreed on a try.
--
Thanks
kalyan
Ph: (408)-585-8040
On 1/21/14, 7:24 AM, Peter Levart wrote:
On 01/21/2014 07:54 AM, Peter Levart wrote:
*[Loaded sun.misc.Cleaner from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]*
[Loaded java.io.ByteArrayInputStream from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$ZoneOffsetTransitionRule
from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
...
I'm on linux, 64bit and using official EA build 121 of JDK 8...
But if I try with JDK 7u45, I don't see it.
So what changed between JDK 7 and JDK 8?
I suspect the following: 8007572: Replace existing jdk timezone data
at java.home/lib/zi with JSR310's tzdb
Regards, Peter
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 01/22/2014 03:19 AM, David Holmes wrote:
Hi Peter,
On 22/01/2014 12:00 AM, Peter Levart wrote:
Hi, David, Kalyan,
Summing up the discussion, I propose the following patch for
ReferenceHandler:
http://cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.01/
I can live with it, though it maybe that once Cleaner has been
preloaded instanceof can no longer throw OOME. Can't be 100% sure. And
there's some duplication/verbosity in the commentary that could be
trimmed down :)
Any specific reason to use Unsafe to do the preload rather than
Class.forName ? Does this force Unsafe to be loaded earlier than it
otherwise would?
Good question. In systemDictionary.hpp they are both on the preloaded
list in this order:
do_klass(Reference_klass, java_lang_ref_Reference,
Pre ) \
...
do_klass(misc_Unsafe_klass,
sun_misc_Unsafe, Pre ) \
So when Reference is initialized, the Unsafe is already loaded. But I
don't know if it is already initialized. This should be studied.
I'll try to find out what is the case and get back to you.
Regards, Peter
Thanks,
David
all 10 java/lang/ref tests pass on my PC (including
OOMEInReferenceHandler).
I kindly ask Kalyan to try to re-run the OOMEInReferenceHandler test
with this code and report any failure.
Thanks, Peter
On 01/21/2014 08:57 AM, David Holmes wrote:
On 21/01/2014 4:54 PM, Peter Levart wrote:
On 01/21/2014 03:22 AM, David Holmes wrote:
Hi Peter,
I do not see Cleaner being loaded prior to the main class on either
Windows or Linux. Which platform are you on? Did you see it loaded
before the main class or as part of executing it?
Before. The main class is empty:
public class Test { public static void main(String... a) {} }
Here's last few lines of -verbose:class:
[Loaded java.util.TimeZone from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfo from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$1 from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.DataInput from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded java.io.DataInputStream from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
*[Loaded sun.misc.Cleaner from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]*
Curious. I wonder what the controlling factor is ??
I'm on linux, 64bit and using official EA build 121 of JDK 8...
But if I try with JDK 7u45, I don't see it. So perhaps it would be
good
to trigger Cleaner loading and initialization as part of
ReferenceHandler initialization to play things safe.
If we do that for Cleaner we may as well do it for
InterruptedException too.
Also, it is not that I think ReferenceHandler is responsible for
reporting OOME, but that it is responsible for reporting that it was
unable to perform a clean or enqueue because of OOME.
This would be necessary if we skipped a Reference because of OOME, but
if we just re-try until we eventually succeed, nothing is lost,
nothing
to report (but a slow response)...
Agreed - just trying to clarify things.
Your suggested approach seems okay though I'm not sure why we
shouldn't help things along by calling System.gc() ourselves rather
than just yielding and hoping things will get cleaned up elsewhere.
But for the present purposes your approach will suffice I think.
Maybe my understanding is wrong but isn't the fact that OOME is
rised a
consequence of that VM has already attempted to clear things up
(executing a GC round synchronously) but didn't succeed to make enough
free space to satisfy the allocation request? If this is only how some
collectors/allocators are implemented and not a general rule, then we
should put a System.gc() in place of Thread.yield(). Should we also
combine that with Thread.yield()? I'm concerned of a possibility
that we
spin, consume too much CPU (ReferenceHandler thread has MAX
priority) so
that other threads dont' get enough CPU time to proceed and clean
things
up (we hope other threads will also get OOME and release things as
their
stacks unwind...).
You are probably right about the System.gc() - OOME should be thrown
after GC fails to create space, so it really needs some other thread
to drop live references to allow further space to be reclaimed.
But note that Thread.yield() can behave badly on some linux systems
too, so spinning is still a possibility - but either way this would
only be really bad on a uniprocessor system where yield() is
unlikely to misbehave.
David
-
Regards, Peter
Thanks,
David
On 20/01/2014 6:42 PM, Peter Levart wrote:
On 01/20/2014 09:00 AM, Peter Levart wrote:
On 01/20/2014 02:51 AM, David Holmes wrote:
Hi Peter,
On 17/01/2014 11:24 PM, Peter Levart wrote:
On 01/17/2014 02:13 PM, Peter Levart
Re: Question about the bug https://bugs.openjdk.java.net/browse/JDK-8031179
Hi Eric, OK, overall this looks good. There are a few adjustments I'd like you to make before I push it for you. Part of this is to get you to do a more complete job of preparing changesets, and part of it is to make my job as a sponsor easier. :-) Oh, and there a couple style issues as well. 1. In the LookupIPv6 test, REGISTRY_PORT is capitalized, making it appear to be a constant, when in fact it's a variable that contains the result of calling getUnusedRandomPort(). It's also unclear why it needs to be a static. Just make it a local variable with a typical variable name, e.g. int port = TestLibrary.getUnusedRandomPort(); and of course make corresponding name changes where it's used. 2. The creation of the URL string for Naming.lookup() concatenates two string literals ] + :. It's probably better to combine these into a single literal ]:. Actually I think it would be preferable to create this URL string using String.format(), so please do this instead. 3. Prepare a complete changeset that's committed locally to your repository before running webrev. This changeset should include properly formatted commit comments, including the bugid line and Reviewed-by line, and optionally a Summary line. Recent versions of webrev will export a mercurial changeset and include it in the webrev output, instead of just diffs. Using the changeset I can just import and push it, instead of having to edit the changeset myself manually. 4. When you post the webrev, you should post updated webrevs under a directory with an incremented sequence number (in this case, webrev.01). That way, links to, and comments on, previous webrevs will not be invalidated. Thanks, s'marks On 1/24/14 1:43 AM, Eric Wang wrote: Hi Stuart, Please review the webrev http://cr.openjdk.java.net/~ewang/JDK-8031179/webrev.00/, if you are OK with the changes, could you please be my sponsor? Thanks, Eric On 2014/1/24 15:14, Eric Wang wrote: Hi Stuart, Thanks for the suggestion! sorry for reply this mail late as i was busy on other tasks The webrev has been in the internal review process. Based on the suggestion, here is a summary of changes: 1. Add othervm options to tests: java/rmi/Naming/DefaultRegistryPort.java java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java java/rmi/Naming/LookupNameWithColon.java java/rmi/server/UnicastRemoteObject/exportObject/GcDuringExport.java sun/rmi/rmic/RMIGenerator/RmicDefault.java java/rmi/MarshalledObject/compare/Compare.java java/rmi/MarshalledObject/compare/HashCode.java 2. Remove java/rmi and sun/rmi from othervm.dirs of TEST.ROOT 3. Filed a another bug https://bugs.openjdk.java.net/browse/JDK-8032629 for exclusiveAccess.dirs Thanks, Eric On 2014/1/18 8:45, Stuart Marks wrote: Hi Eric, Thanks for doing the analysis of the tests that need /othervm. The list of tests that don't need /othervm looks good. One subtlety is this one: java/rmi/activation/ActivationGroupDesc/checkDefaultGroupName/CheckDefaultGroupName.java Most activation tests do need /othervm, but this one doesn't, since all it does is create an ActivationGroupDesc instance, which has no side effects. This is unusual for the activation APIs, since most of them are quite intertwined with the rest of the activation subsystem. So, for this test, the lack of /othervm warrants a comment explaining why /othervm is unnecessary. Regarding merging of the tests java/rmi/MarshalledObject/compare/Compare.java and HashCode.java, this is fairly subtle and not obviously wrong, but it's not obviously right either. Note that different jtreg tests -- even in agentvm or samevm mode -- load the test class in a fresh classloader, which means that the statics are reinitialized. This provides some degree of isolation of the tests even when they're reusing the same JVM. By contrast, calling the two different methods from within the same test exposes the second one to initializations left from the first one. In addition (though I'm not too concerned about this) if the first test fails the second won't be run at all. Merging these tests is kind of out of scope for this particular bug, and since I wasn't fully able to convince myself that the merged tests have the same semantics as the unmerged tests, I'd prefer not to see the merging of these tests as part of this changeset. (Some cleanup of these two tests is probably warranted eventually. I'd take a different approach of merging and refactoring the sources but keeping them as separate test runs, using two @run tags. But we should work on that separately. Meanwhile, the overhead of having these as separate tests is minimal, especially if they're not run in /othervm mode.) I don't think the removal of java/rmi/Naming from exclusiveAccess.dirs is safe at this point. The DefaultRegistryPort.java test uses the default registry port, not a unique one. Conceptually it would need to be converted to use the TestLibrary unique port stuff, but the test is actually about using the
Re: RFR: JDK-8032050: TEST_BUG: java/rmi/activation/Activatable/shutdownGracefully/ShutdownGracefully.java fails intermittently
On 1/23/14 10:34 PM, Tristan Yan wrote: Hi All Could you review the bug fix for JDK-8032050. http://cr.openjdk.java.net/~tyan/JDK-8032050/webrev.00/ Description: This rare happened failure caused because when RMID starts. It don't guarantee sun.rmi.server.Activation.startActivation finishes. Fix by adding a iterative getSystem with a 5 seconds timeout. Hi Tristan, Adding a timing/retry loop into this test isn't the correct approach for fixing this test. The timing loop isn't necessary because there is already a timing loop in RMID.start() in the RMI test library. (There's another timing loop in ActivationLibrary.rmidRunning() which should probably be removed.) So the intent of this library call is that it start rmid and wait for it to become ready. That logic doesn't need to be added to the test. In the bug report JDK-8032050 you had mentioned that the NullPointerException was suspicious. You're right! I took a look and it seemed like it was related to JDK-8023541, and I added a note to this effect to the bug report. The problem here is that rmid can come up and transiently return null instead of the stub of the activation system. That's what JDK-8023541 covers. I think that rmid itself needs to be fixed, though modifying the timing loop in the RMI test library to wait for rmid to come up *and* for the lookup to return non-null is an easy way to fix the problem. (Or at least cover it up.) The next step in the analysis is to determine, given that ActivationLibrary.getSystem can sometimes return null, whether this has actually caused this test failure. This is pretty easy to determine; just hack in a line system = null in the right place and run the test. I've done this, and the test times out and the output log is pretty much identical to the one in the bug report. (I recommend you try this yourself.) So I think it's fairly safe to say that the problem in JDK-8023541 has caused the failure listed in JDK-8032050. I can see a couple ways to proceed here. One way is just to close this out as a duplicate of JDK-8023541 since that bug caused this failure. Another is that this test could use some cleaning up. This bug certainly covers a failure, but the messages emitted are confusing and in some cases completely wrong. For example, the rmid has shutdown message at line 180 is incorrect, because in this case rmid is still running and the wait() call has timed out. Most of the code here can be replaced with calls to various bits of the RMI test library. There are a bunch of other things in this test that could be cleaned up as well. It's up to you how you'd like to proceed. s'marks
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
Hi Peter, if you are a committer would you like to take this further
(OR) perhaps david could sponsor this change.
--
Thanks
kalyan
On 1/24/14 4:05 PM, Peter Levart wrote:
On 01/24/2014 02:53 AM, srikalyan chandrashekar wrote:
Hi David, yes thats right, only benefit i see is we can avoid
assignment to 'r' if pending is null.
Hi Kalyan,
Good to hear that test runs without failures so far.
Regarding assignment of 'r'. What I tried to accomplish with the
change was eliminate double reading of 'pending' field. I have a
mental model of local variable being a register and field being a
memory location. This may be important if the field is volatile, but
for normal fields, I guess the optimizer knows how to compile such
code most optimally in either case. The old (your) version is better
from logical perspective, since it guarantees that dereferencing the
'r', wherever it is possible, will never throw NPE (dereferencing
where 'r' is not assigned is not possible because of definitive
assignment rules). So I support going back to your version...
Regards, Peter
--
Thanks
kalyan
On 1/23/14 4:33 PM, David Holmes wrote:
On 24/01/2014 6:10 AM, srikalyan wrote:
Hi Peter, i have modified your code from
r = pending;
if (r != null) {
..
TO
if (pending != null) {
r = pending;
This is because the r is used later in the code and must not be
assigned
pending unless it is not null(this was as is earlier).
If r is null, because pending is null then you perform the wait()
and then continue - back to the top of the loop. There is no bug in
Peter's code.
The new webrev is
posted here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev-V2/
. I ran a 1000 run and no failures so far, however i would like to
run a
couple more 1000 runs to assert the fix.
PS: The description section of JEP-122
(http://openjdk.java.net/jeps/122) says meta-data would be in native
memory(not heap).
The class_mirror is a Java object not meta-data.
David
--
Thanks
kalyan
Ph: (408)-585-8040
On 1/21/14, 2:31 PM, Peter Levart wrote:
On 01/21/2014 07:17 PM, srikalyan wrote:
Hi Peter/David, catching up after long weekend. Why would there
be an
OOME in object heap due to class loading in perm gen space ?
The perm gen is not a problem her (JDK 8 does not have it and we see
OOME on JDK8 too). Each time a class is loaded, new java.lang.Class
object is allocated on heap.
Regards, Peter
Please correct if i am missing something here. Meanwhile i will give
the version of Reference Handler you both agreed on a try.
--
Thanks
kalyan
Ph: (408)-585-8040
On 1/21/14, 7:24 AM, Peter Levart wrote:
On 01/21/2014 07:54 AM, Peter Levart wrote:
*[Loaded sun.misc.Cleaner from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]*
[Loaded java.io.ByteArrayInputStream from
/home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
[Loaded sun.util.calendar.ZoneInfoFile$ZoneOffsetTransitionRule
from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
...
I'm on linux, 64bit and using official EA build 121 of JDK 8...
But if I try with JDK 7u45, I don't see it.
So what changed between JDK 7 and JDK 8?
I suspect the following: 8007572: Replace existing jdk timezone
data
at java.home/lib/zi with JSR310's tzdb
Regards, Peter
