Correction ...
On 21/09/2020 4:16 pm, David Holmes wrote:
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn <r...@openjdk.org> wrote:
This patch implements asynchronous handshake, which changes how handshakes
works by default. Asynchronous handshakes
are target only executed, which they may never be executed. (target may block
on socket for the rest of VM lifetime)
Since we have several use-cases for them we can have many handshake pending.
(should be very rare) To be able handle an
arbitrary amount of handshakes this patch adds a per JavaThread queue and heap
allocated HandshakeOperations. It's a
singly linked list where you push/insert to the end and pop/get from the front.
Inserts are done via CAS on first
pointer, no lock needed. Pops are done while holding the per handshake state
lock, and when working on the first
pointer also CAS. The thread grabbing the handshake state lock for a
JavaThread will pop and execute all handshake
operations matching the filter. The JavaThread itself uses no filter and any
other thread uses the filter of everything
except asynchronous handshakes. In this initial change-set there is no need to
do any other filtering. If needed
filtering can easily be exposed as a virtual method on the HandshakeClosure,
but note that filtering causes handshake
operation to be done out-order. Since the filter determins who execute the
operation and not the invoked method, there
is now only one method to call when handshaking one thread. Some comments
about the changes:
- HandshakeClosure uses ThreadClosure, since it neat to use the same closure
for both alla JavThreads do and Handshake
all threads. With heap allocating it cannot extends StackObj. I tested
several ways to fix this, but those very much
worse then this.
- I added a is_handshake_safe_for for checking if it's current thread is
operating on itself or the handshaker of that
thread.
- Simplified JVM TI with a JvmtiHandshakeClosure and also made them not needing
a JavaThread when executing as a
handshaker on a JavaThread, e.g. VM Thread can execute the handshake
operation.
- Added WB testing method.
- Removed VM_HandshakeOneThread, the VM thread uses the same call path as
direct handshakes did.
- Changed the handshake semaphores to mutex to be able to handle deadlocks with
lock ranking.
- VM_HandshakeAllThreadsis still a VM operation, since we do support half of
the threads being handshaked before a
safepoint and half of them after, in many handshake all operations.
- ThreadInVMForHandshake do not need to do a fenced transistion since this is
always a transistion from unsafe to unsafe.
- Added NoSafepointVerifyer, we are thinking about supporting safepoints inside
handshake, but it's not needed at the
moment. To make sure that gets well tested if added the NoSafepointVerifyer
will raise eyebrows.
- Added ttyLocker::break_tty_lock_for_safepoint(os::current_thread_id()); due
to lock rank.
- Added filtered queue and gtest for it.
Passes multiple t1-8 runs.
Been through some pre-reviwing.
Robbin Ehn has updated the pull request incrementally with one additional
commit since the last revision:
Fixed double checks
Added NSV
ProcessResult to enum
Fixed logging
Moved _active_handshaker to private
src/hotspot/share/runtime/handshake.cpp line 336:
334: // and thus prevents reading stale data modified in the handshake closure
335: // by the Handshakee.
336: OrderAccess::acquire();
How/why is this deleted? Surely there are still single-thread VMops that use a
handshake??
That comment was placed against the old line 336 which was the deletion
of this method:
bool Handshake::execute(HandshakeClosure* thread_cl, JavaThread* target) {
(I'll file a skara/git bug).
David
-----
src/hotspot/share/runtime/interfaceSupport.inline.hpp line 136:
134: assert(_thread->thread_state() == _thread_in_vm, "should only call when
leaving VM after handshake");
135:
136: _thread->set_thread_state(_original_state);
Can you clarify why this is no longer needed? What states can we be returning
to?
-------------
PR: https://git.openjdk.java.net/jdk/pull/151
