Re: RFR: 8035939: java/lang/management/MemoryMXBean/MemoryManagement.java timed out on Linux-amd64
Looks good! Thanks, /Staffan On 19 aug 2014, at 12:25, Stefan Karlsson stefan.karls...@oracle.com wrote: Hi all, Please review this patch harden two MemoryMXBean tests. These tests cause intermittent test failures when the allocated objects are put in the young gen instead of the old gen. The proposed fix/workaround is to lower the young gen size and assert that the allocated objects are large enough to not fit in the young gen. http://cr.openjdk.java.net/~stefank/8035939/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8035939 thanks, StefanK
Re: RFR: 8035939: java/lang/management/MemoryMXBean/MemoryManagement.java timed out on Linux-amd64
Hi Stefank, Seems reasonable. Reviewed. Bengt On 2014-08-19 12:25, Stefan Karlsson wrote: Hi all, Please review this patch harden two MemoryMXBean tests. These tests cause intermittent test failures when the allocated objects are put in the young gen instead of the old gen. The proposed fix/workaround is to lower the young gen size and assert that the allocated objects are large enough to not fit in the young gen. http://cr.openjdk.java.net/~stefank/8035939/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8035939 thanks, StefanK
Re: RFR: 8035939: java/lang/management/MemoryMXBean/MemoryManagement.java timed out on Linux-amd64
On 2014-08-19 12:51, Staffan Larsen wrote: Looks good! Thanks! StefanK Thanks, /Staffan On 19 aug 2014, at 12:25, Stefan Karlsson stefan.karls...@oracle.com wrote: Hi all, Please review this patch harden two MemoryMXBean tests. These tests cause intermittent test failures when the allocated objects are put in the young gen instead of the old gen. The proposed fix/workaround is to lower the young gen size and assert that the allocated objects are large enough to not fit in the young gen. http://cr.openjdk.java.net/~stefank/8035939/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8035939 thanks, StefanK
Re: RFR: 8035939: java/lang/management/MemoryMXBean/MemoryManagement.java timed out on Linux-amd64
Hi Stefan, On 2014-08-19 12:25, Stefan Karlsson wrote: Hi all, Please review this patch harden two MemoryMXBean tests. These tests cause intermittent test failures when the allocated objects are put in the young gen instead of the old gen. The proposed fix/workaround is to lower the young gen size and assert that the allocated objects are large enough to not fit in the young gen. http://cr.openjdk.java.net/~stefank/8035939/webrev.00/ Looks good. /Mikael https://bugs.openjdk.java.net/browse/JDK-8035939 thanks, StefanK
Re: RFR [8055338]: (process) Add instrumentation to help diagnose JDK-6573254
Hello again! I updated the patch to cover situations when the exiting thread isn't daemon. I also added load_acquire/store_release for the sake of accuracy, even though on Windows they seem to add nothing to the volatile access. http://cr.openjdk.java.net/~igerasim/8055338/1/webrev/ If the updated patch looks Okay, I'll need a sponsor to push it. Sincerely yours, Ivan On 19.08.2014 6:42, David Holmes wrote: On 19/08/2014 10:12 AM, Ioi Lam wrote: With the Windows/x86/x64 memory model, is the write to vm_getting_terminated guaranteed to be observable by java_start()? In the general case, not immediately. For the threads actually of interest the logic that tells the threads to terminate happens after the write to the flag, and that logic contains sufficient synchronization that if the termination logic is correct then the flag must also be visible. David - Ioi On 8/18/14, 2:19 PM, Ivan Gerasimov wrote: Hello! This is a request to temporarily add some instrumentation code to hotspot to help diagnose the intermittent failure on Windows, which results in a wrong exit code of (sub-)process. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055338 WEBREV: http://cr.openjdk.java.net/~igerasim/8055338/0/webrev/ Sincerely yours, Ivan
Re: RFR [8055338]: (process) Add instrumentation to help diagnose JDK-6573254
Is there any reason why this link is only accessible if I log-in? Mario 2014-08-18 23:19 GMT+02:00 Ivan Gerasimov ivan.gerasi...@oracle.com: Hello! This is a request to temporarily add some instrumentation code to hotspot to help diagnose the intermittent failure on Windows, which results in a wrong exit code of (sub-)process. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055338 WEBREV: http://cr.openjdk.java.net/~igerasim/8055338/0/webrev/ Sincerely yours, Ivan -- pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF Blog: http://neugens.wordpress.com - Twitter: @neugens Proud GNU Classpath developer: http://www.classpath.org/ Read About us at: http://planet.classpath.org OpenJDK: http://openjdk.java.net/projects/caciocavallo/ Please, support open standards: http://endsoftpatents.org/
Re: RFR [8055338]: (process) Add instrumentation to help diagnose JDK-6573254
This is a sub-task and it inherits its security level from the parent bug. It appears that Jira doesn't allow editing the security level of sub-tasks. Sincerely yours, Ivan On 19.08.2014 17:16, Mario Torre wrote: Is there any reason why this link is only accessible if I log-in? Mario 2014-08-18 23:19 GMT+02:00 Ivan Gerasimov ivan.gerasi...@oracle.com: Hello! This is a request to temporarily add some instrumentation code to hotspot to help diagnose the intermittent failure on Windows, which results in a wrong exit code of (sub-)process. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8055338 WEBREV: http://cr.openjdk.java.net/~igerasim/8055338/0/webrev/ Sincerely yours, Ivan
RFR(S): URG! JDK-8049226 com/sun/jdi/OptionTest.java test times out again
Hi Everybody, Please review the fix: http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.01/ JDWP call jniFatalError if transport can't be initialized (e.g. wrong parameters) and jniFatalError call os::abort(). Therefor all transport initialization errors cause vm to coredump. I see no reason for debugInit_exit to call jniFatalError so remove this code. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR(S): URG! JDK-8049226 com/sun/jdi/OptionTest.java test times out again
Hi Dmitry, The fix seems to be Ok. Just want to make it clear... This fix just changes the bug pattern. It a case of incorrect transport parameters the test is still going to fail but without crash, right? Thanks, Serguei On 8/19/14 12:09 PM, Dmitry Samersoff wrote: Hi Everybody, Please review the fix: http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.01/ JDWP call jniFatalError if transport can't be initialized (e.g. wrong parameters) and jniFatalError call os::abort(). Therefor all transport initialization errors cause vm to coredump. I see no reason for debugInit_exit to call jniFatalError so remove this code. -Dmitry
Re: RFR 8055008: Clean up code that saves the previous versions of redefined classes
On 8/15/14 2:18 PM, Coleen Phillimore wrote:
Summary: Use scratch_class to find EMCP methods for breakpoints if the
old methods are still running
See bug for more details. This fix also includes a change to
nmethod::metadata_do() to not walk the _method multiple times (the
_method is added to the metadata section of the nmethod), and an
attempt to help the sweeper clean up these scratch_class instance
classes sooner.
Tested with nsk tests, java/lang/instrument tests and jck tests (which
include some jvmti tests).
open webrev at http://cr.openjdk.java.net/~coleenp/8055008/
src/share/vm/oops/instanceKlass.hpp
line 1047 // RedefineClass support
Should be 'RedefineClasses'.
line 1049: void mark_newly_obsolete_methods(ArrayMethod**
old_methods,
int emcp_method_count);
The 'obsolete' part of the function name does not match with
the 'emcp' part of the parameter name. EMCP methods are 'old',
but not 'obsolete'.
Update: OK, I think I get it. We want to mark methods that are
newly transitioning from EMCP - obsolete and the emcp_method_count
parameter tells us if there is any work to do.
src/share/vm/oops/instanceKlass.cpp
line 3023: } // pvw is cleaned up
'pvw' no longer exists so comment is stale.
line 3455: // check the previous versions array
This comment should move above this line:
3453 for (; pv_node != NULL; ) {
and 'array' should change to 'list'.
Sorry for the bad placement of the original comment.
line 3463: last-link_previous_versions(pv_node);
So now we've overwritten the previous value of
last-previous_versions. How does that InstanceKlass
get freed? Maybe a short comment?
line 3484: // Mark the emcp method as obsolete if it's not executing
I'm not sure about whether this is a good idea. When we had a
redefine make a method obsolete before, we knew that we could
make all older but previously EMCP methods obsolete because
the new method version did make them obsolete.
With this optimization, we're saying that no thread is executing
the method so we're going to make it obsolete even if the current
redefine did not do so. I worry about a method call that is in
the early stages of assembling the call stack being caught
calling a method that is now obsolete but not because of a
redefine made it obsolete. Just FYI, one of the tracing flags
catches unexpected calls to obsolete methods today and it does
catch the current system's legitimate race.
line 3527: // clear out any matching EMCP method entries the hard way.
Perhaps mark instead of clear out?
old line 3659: if (!method-is_obsolete()
new line 3546: if (method-is_emcp()
The old code is correct. The old code won't remark a method that
was already made obsolete so there won't be more than one trace
message for that operation.
line 3581: // stack, and set emcp methods on the stack.
In comments 'emcp' should be 'EMCP'. We did not do that in the
code because of HotSpot's variable name style.
Also, set what on EMCP methods on the stack?
line 3591: ... If emcp method from
line 3592: // a previous redefinition may be made obsolete by this
redefinition.
Having trouble parsing this comment.
src/share/vm/oops/method.hpp
line 693: // emcp methods (equivalent method except constant pool
is different)
line 694: // that are old but not obsolete or deleted.
Perhaps:
// EMCP methods are old but not obsolete or deleted. Equivalent
// Modulo Constant Pool means the method is equivalent except
// the constant pool and instructions that access the constant
// pool might be different.
src/share/vm/prims/jvmtiImpl.cpp
No comments.
src/share/vm/prims/jvmtiRedefineClasses.cpp
No comments.
src/share/vm/code/nmethod.cpp
So in the original code f(_method) was being called two extra
times? (once in the while-loop and once at the end) So I'm
guessing that f(_method) is properly called when the rest of
the metadata is handled in the nmethod (line 2085)?
src/share/vm/memory/universe.cpp
No comments (resisting 'The Walking Dead' ref...)
test/runtime/RedefineTests/RedefineFinalizer.java
No comments.
test/runtime/RedefineTests/RedefineRunningMethods.java
line 44: while (!stop) { count2++; } +
line 53: while (!stop) { count1++; }
line 56: while (!stop) { count2++; }
These may not behave well on OSes with bad threading
models. You might want to add a helper function that
sleeps for 10ms and have each of these loops call it
so the test more well behaved.
Dan
bug link https://bugs.openjdk.java.net/browse/JDK-8055008
Thanks,
Coleen
Re: RFR 8055008: Clean up code that saves the previous versions of redefined classes
On 8/19/14 3:53 PM, Daniel D. Daugherty wrote:
On 8/19/14 3:39 PM, Daniel D. Daugherty wrote:
On 8/15/14 2:18 PM, Coleen Phillimore wrote:
Summary: Use scratch_class to find EMCP methods for breakpoints if
the old methods are still running
See bug for more details. This fix also includes a change to
nmethod::metadata_do() to not walk the _method multiple times (the
_method is added to the metadata section of the nmethod), and an
attempt to help the sweeper clean up these scratch_class instance
classes sooner.
Tested with nsk tests, java/lang/instrument tests and jck tests
(which include some jvmti tests).
open webrev at http://cr.openjdk.java.net/~coleenp/8055008/
src/share/vm/oops/instanceKlass.hpp
line 1047 // RedefineClass support
Should be 'RedefineClasses'.
line 1049: void mark_newly_obsolete_methods(ArrayMethod**
old_methods,
int emcp_method_count);
The 'obsolete' part of the function name does not match with
the 'emcp' part of the parameter name. EMCP methods are 'old',
but not 'obsolete'.
Update: OK, I think I get it. We want to mark methods that are
newly transitioning from EMCP - obsolete and the
emcp_method_count
parameter tells us if there is any work to do.
src/share/vm/oops/instanceKlass.cpp
line 3023: } // pvw is cleaned up
'pvw' no longer exists so comment is stale.
line 3455: // check the previous versions array
This comment should move above this line:
3453 for (; pv_node != NULL; ) {
and 'array' should change to 'list'.
Sorry for the bad placement of the original comment.
line 3463: last-link_previous_versions(pv_node);
So now we've overwritten the previous value of
last-previous_versions. How does that InstanceKlass
get freed? Maybe a short comment?
line 3484: // Mark the emcp method as obsolete if it's not executing
I'm not sure about whether this is a good idea. When we had a
redefine make a method obsolete before, we knew that we could
make all older but previously EMCP methods obsolete because
the new method version did make them obsolete.
With this optimization, we're saying that no thread is executing
the method so we're going to make it obsolete even if the
current
redefine did not do so. I worry about a method call that is in
the early stages of assembling the call stack being caught
calling a method that is now obsolete but not because of a
redefine made it obsolete. Just FYI, one of the tracing flags
catches unexpected calls to obsolete methods today and it does
catch the current system's legitimate race.
JVM/TI has an isMethodObsolete() API:
jvmtiError
IsMethodObsolete(jvmtiEnv* env,
jmethodID method,
jboolean* is_obsolete_ptr)
It would be possible for an agent to observe a method changing from
not obsolete to obsolete when that's not expected. I suspect that
this would be a spec violation.
I agree that this looks like a spec violation.
The emcp methods by definition are non-obsolete,
or in opposite, the obsolete methods are non-emcp:
http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#obsoleteMethods
Old method versions may become obsolete, not emcp:
http://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#RedefineClasses
But maybe I'm missing something...
Thanks,
Serguei
Dan
line 3527: // clear out any matching EMCP method entries the hard
way.
Perhaps mark instead of clear out?
old line 3659: if (!method-is_obsolete()
new line 3546: if (method-is_emcp()
The old code is correct. The old code won't remark a method that
was already made obsolete so there won't be more than one trace
message for that operation.
line 3581: // stack, and set emcp methods on the stack.
In comments 'emcp' should be 'EMCP'. We did not do that in the
code because of HotSpot's variable name style.
Also, set what on EMCP methods on the stack?
line 3591: ... If emcp method from
line 3592: // a previous redefinition may be made obsolete by
this redefinition.
Having trouble parsing this comment.
src/share/vm/oops/method.hpp
line 693: // emcp methods (equivalent method except constant pool
is different)
line 694: // that are old but not obsolete or deleted.
Perhaps:
// EMCP methods are old but not obsolete or deleted. Equivalent
// Modulo Constant Pool means the method is equivalent except
// the constant pool and instructions that access the constant
// pool might be different.
src/share/vm/prims/jvmtiImpl.cpp
No comments.
src/share/vm/prims/jvmtiRedefineClasses.cpp
No comments.
src/share/vm/code/nmethod.cpp
So in the original code f(_method) was being called two extra
