RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]
Hi all, I worked on updating my script and handling more assignments so I redid a second pass on the same files to get all the cases. Now, on those files the regex "if.* = " no longer shows any cases we would want to fix. Could I get a review for this webrev: Webrev: http://cr.openjdk.java.net/~jcbeyler/8213003/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8213003 I tested this on my dev machine by passing all the tests that were modified. Thanks! Jc
Re: RFR 8195627: [Graal] nsk/jdi/VirtualMachine/redefineClasses/redefineclasses026 hangs with Graal in Xcomp mode
Hi Daniil, It looks good. Thanks! Serguei On 10/29/18 11:13 AM, Daniil Titov wrote: Please review the change that fixes the issue when the test hangs with Graal and XComp mode on. One of the test cases the test runs is to call com.sun.jdi.VirtualMachine.redefineClasses() while the target VM is suspended. The bytes passed to com.sun.jdi.VirtualMachine.redefineClasses() don't correspond to the reference type (the names do not match) and the test expects NoClassDefFoundError to be thrown. The problem here is that the compiler thread is suspended while "JDWP Transport Listener" thread enters CompileBroker::wait_for_jvmci_completion() method and the code inside CompileBroker::wait_for_jvmci_completion() method falsely assumes that some progress by the compiler thread was made. As a result, it resets the progress wait attempt counter and goes into an endless loop waiting for JVMCI to complete the compilation. Webrev: http://cr.openjdk.java.net/~dtitov/8195627/webrev.01/ Issue: https://bugs.openjdk.java.net/browse/JDK-8195627 Thanks! --Daniil
Re: RFR 8195627: [Graal] nsk/jdi/VirtualMachine/redefineClasses/redefineclasses026 hangs with Graal in Xcomp mode
Looks good. Thanks, Vladimir On 10/30/18 2:04 AM, serguei.spit...@oracle.com wrote: The Compiler team needs to review this as well, so added hotspot-compiler-dev mailing list. Thanks, Serguei On 10/29/18 11:13, Daniil Titov wrote: Please review the change that fixes the issue when the test hangs with Graal and XComp mode on. One of the test cases the test runs is to call com.sun.jdi.VirtualMachine.redefineClasses() while the target VM is suspended. The bytes passed to com.sun.jdi.VirtualMachine.redefineClasses() don't correspond to the reference type (the names do not match) and the test expects NoClassDefFoundError to be thrown. The problem here is that the compiler thread is suspended while "JDWP Transport Listener" thread enters CompileBroker::wait_for_jvmci_completion() method and the code inside CompileBroker::wait_for_jvmci_completion() method falsely assumes that some progress by the compiler thread was made. As a result, it resets the progress wait attempt counter and goes into an endless loop waiting for JVMCI to complete the compilation. Webrev: http://cr.openjdk.java.net/~dtitov/8195627/webrev.01/ Issue: https://bugs.openjdk.java.net/browse/JDK-8195627 Thanks! --Daniil
Re: RFR: 8212933: Thread-SMR: requesting a VM operation whilst holding a ThreadsListHandle can cause deadlocks
On 10/29/18 9:31 PM, David Holmes wrote:
Thanks for the explanation Robbin.
The inline patch also seems fine. I hope the other reviewers noticed it.
Yes, but I forgot to reply to it.
Thumbs up.
Dan
David
On 29/10/2018 7:05 PM, Robbin Ehn wrote:
Hi David,
On 29/10/2018 07:20, David Holmes wrote:
Hi Robbin,
On 29/10/2018 6:08 AM, Robbin Ehn wrote:
Hi Dan,
Thanks for looking at this, here is the update:
Inc: http://cr.openjdk.java.net/~rehn/8212933/v2/inc/webrev/
Full: http://cr.openjdk.java.net/~rehn/8212933/v2/webrev/
I can't say I really understand the change in protocol here and why
all the cancel operations are no longer needed. I see the handshake
VM operations reusing the initial "threads list" but I'm unclear how
they might be affected if additional threads are added to the system
before the Threads_lock is acquired?
The ThreadsList is a snapshot of all the JavaThreads at that time in
the VM.
Handshake all threads only handshake those JavaThreads. We do not
care about new
threads.
The typical generic use-case is the similar to RCU. You first update
a global
state and emit the handshake when the handshake return no thread can
see the old
state.
GlobalFuncPtr = some_new_func;
HandshakeAllThreads;
--
No thread can be executing the old func.
If the JavaThreads have a local copy of GlobalFuncPtr the handshake
operation would be to update the local copy to some_new_func.
It works for both Java and for VM resources that respect safepoints.
For a pure VM resource it's much cheaper to use the GlobalCounter.
The Threads_lock must only be held for S/R protocol.
With changes to the S/R protocol, such as using handshake instead, we
can remove
Threads_lock for handshakes completely. (with a other small fixes)
The cancel is no longer needed since the terminated threads are
visible to the
VM thread when we keep the arming threadslist. We add terminated
threads as safe
for handshake. But if we handshake a terminated thread we do not
execute the
handshake operation, instead just clear the operation and increment the
completed counter. (the VM thread cancels the operation)
I hope that helped?
A couple of specific comments:
src/hotspot/share/runtime/handshake.hpp
cancel_inner() is dead now.
---
src/hotspot/share/runtime/handshake.cpp
This was an odd looking for loop before your change and now looks
even more strange:
for ( ; JavaThread *thr = jtiwh.next(); ) {
can it not simply be a more normal looking:
for (JavaThread *thr = jtiwh.next(); thr != NULL; thr =
jtiwh.next()) {
?
Thanks, fixed with below patch.
/Robbin
diff -r 5f8b292c473f src/hotspot/share/runtime/handshake.cpp
--- a/src/hotspot/share/runtime/handshake.cpp Sun Oct 28 20:57:24
2018 +0100
+++ b/src/hotspot/share/runtime/handshake.cpp Mon Oct 29 09:32:26
2018 +0100
@@ -166,1 +166,1 @@
- for ( ; JavaThread *thr = jtiwh.next(); ) {
+ for (JavaThread *thr = jtiwh.next(); thr != NULL; thr =
jtiwh.next()) {
@@ -198,1 +198,1 @@
- for ( ; JavaThread *thr = jtiwh.next(); ) {
+ for (JavaThread *thr = jtiwh.next(); thr != NULL; thr =
jtiwh.next()) {
diff -r 5f8b292c473f src/hotspot/share/runtime/handshake.hpp
--- a/src/hotspot/share/runtime/handshake.hpp Sun Oct 28 20:57:24
2018 +0100
+++ b/src/hotspot/share/runtime/handshake.hpp Mon Oct 29 09:32:26
2018 +0100
@@ -63,1 +62,0 @@
- void cancel_inner(JavaThread* thread);
---
Thanks,
David
/Robbin
On 26/10/2018 17:38, Daniel D. Daugherty wrote:
On 10/26/18 10:33 AM, Robbin Ehn wrote:
Hi, please review.
When the VM thread executes a handshake it uses different
ThreadsLists during
the execution. A JavaThread that is armed for the handshake when
it is already
in the exit path in VM will cancel the handshake. Even if the VM
thread cannot
see this thread after the initial ThreadsList which where used
for arming, the
handshake can progress when the exiting thread cancels the
handshake.
But if a third thread takes a ThreadsList where the exiting
JavaThread is present and tries to execute a VM operation, hence
waiting on VM thread to finish the handshake, the JavaThread in
the exit path can never reach the handshake cancellation point.
VM thread cannot finishes the handshake and the third thread is
stuck waiting on the VM thread.
To allow holding a ThreadsList when executing a VM operation we
instead let the
VM thread use the same ThreadsList over the entire handshake
making all armed
threads visible to the VM thread at all time. And if VM thread
spots a terminated thread it will count that thread is already
done by only clearing
it's operation.
Passes local stress testing, t1-5 and the deadlock is no longer
reproduce-able.
Added a jtreg handshake + thread suspend test as a reproducer.
Issue: https://bugs.openjdk.java.net/browse/JDK-8212933
Code: http://cr.openjdk.java.net/~rehn/8212933/v1/webrev/
src/hotspot/share/runtime/handshake.hpp
No comments
Re: RFR 8195627: [Graal] nsk/jdi/VirtualMachine/redefineClasses/redefineclasses026 hangs with Graal in Xcomp mode
The Compiler team needs to review this as well, so added hotspot-compiler-dev mailing list. Thanks, Serguei On 10/29/18 11:13, Daniil Titov wrote: Please review the change that fixes the issue when the test hangs with Graal and XComp mode on. One of the test cases the test runs is to call com.sun.jdi.VirtualMachine.redefineClasses() while the target VM is suspended. The bytes passed to com.sun.jdi.VirtualMachine.redefineClasses() don't correspond to the reference type (the names do not match) and the test expects NoClassDefFoundError to be thrown. The problem here is that the compiler thread is suspended while "JDWP Transport Listener" thread enters CompileBroker::wait_for_jvmci_completion() method and the code inside CompileBroker::wait_for_jvmci_completion() method falsely assumes that some progress by the compiler thread was made. As a result, it resets the progress wait attempt counter and goes into an endless loop waiting for JVMCI to complete the compilation. Webrev: http://cr.openjdk.java.net/~dtitov/8195627/webrev.01/ Issue: https://bugs.openjdk.java.net/browse/JDK-8195627 Thanks! --Daniil
RE: RFR : JDK-8211951 - Broken links in java.management files
Thanks Serguei & Alan for the reviews. Amit From: Serguei Spitsyn Sent: Tuesday, October 30, 2018 12:53 AM To: Alan Bateman; Amit Sapre; serviceability-dev@openjdk.java.net Subject: Re: RFR : JDK-8211951 - Broken links in java.management files Hi Amit, +1 Thanks, Serguei On 10/28/18 23:43, Alan Bateman wrote: On 29/10/2018 06:08, Amit Sapre wrote: Hello, Please review the docs fixes for broken links. Bug ID : https://bugs.openjdk.java.net/browse/JDK-8211951 Webrev : HYPERLINK "http://cr.openjdk.java.net/%7Easapre/webrev/2018/JDK-8211951/00/webrev/"http://cr.openjdk.java.net/~asapre/webrev/2018/JDK-8211951/00/webrev/ Looks okay.
