--- old/src/hotspot/share/runtime/biasedLocking.cpp 2020-01-15 18:32:10.857788229 +0000 +++ new/src/hotspot/share/runtime/biasedLocking.cpp 2020-01-15 18:32:10.019768312 +0000 @@ -622,7 +622,7 @@ p2i(biaser), p2i(obj())); RevokeOneBias revoke(obj, requester, biaser); - bool executed = Handshake::execute(&revoke, biaser, true); + bool executed = Handshake::execute_direct(&revoke, biaser); if (revoke.status_code() == NOT_REVOKED) { return NOT_REVOKED; } --- old/src/hotspot/share/runtime/handshake.cpp 2020-01-15 18:32:11.817811046 +0000 +++ new/src/hotspot/share/runtime/handshake.cpp 2020-01-15 18:32:10.981791176 +0000 @@ -45,6 +45,7 @@ bool _is_direct; public: HandshakeOperation(HandshakeClosure* cl) : _handshake_cl(cl), _pending_threads(1), _executed(false), _is_direct(false) {} + HandshakeOperation(HandshakeClosure* cl, bool is_direct) : _handshake_cl(cl), _pending_threads(1), _executed(false), _is_direct(is_direct) {} void do_handshake(JavaThread* thread); bool is_completed() { assert(_pending_threads >= 0, "_pending_threads cannot be negative"); @@ -54,7 +55,6 @@ bool executed() const { return _executed; } const char* name() { return _handshake_cl->name(); } - void set_isdirect() { _is_direct = true; } bool is_direct() { return _is_direct; } #ifdef ASSERT @@ -254,8 +254,8 @@ void Handshake::execute(HandshakeClosure* thread_cl) { if (SafepointMechanism::uses_thread_local_poll()) { - HandshakeOperation cto(thread_cl); - VM_HandshakeAllThreads handshake(&cto); + HandshakeOperation ho(thread_cl); + VM_HandshakeAllThreads handshake(&ho); VMThread::execute(&handshake); } else { VM_HandshakeFallbackOperation op(thread_cl); @@ -263,16 +263,12 @@ } } -bool Handshake::execute(HandshakeClosure* thread_cl, JavaThread* target, bool is_direct_handshake) { +bool Handshake::execute(HandshakeClosure* thread_cl, JavaThread* target) { if (SafepointMechanism::uses_thread_local_poll()) { HandshakeOperation ho(thread_cl); - if (is_direct_handshake) { - direct_handshake(target, &ho); - } else { - VM_HandshakeOneThread op(&ho, target); - VMThread::execute(&op); - } - return ho.executed(); + VM_HandshakeOneThread handshake(&ho, target); + VMThread::execute(&handshake); + return handshake.executed(); } else { VM_HandshakeFallbackOperation op(thread_cl, target); VMThread::execute(&op); @@ -280,9 +276,9 @@ } } -void Handshake::direct_handshake(JavaThread* target, HandshakeOperation *op) { +bool Handshake::execute_direct(HandshakeClosure* thread_cl, JavaThread* target) { JavaThread *self = (JavaThread*)Thread::current(); - op->set_isdirect(); + HandshakeOperation op(thread_cl, true); jlong start_time_ns = 0; if (log_is_enabled(Info, handshake)) { @@ -291,23 +287,25 @@ ThreadsListHandle tlh; if (tlh.includes(target)) { - target->set_handshake_operation(op); + target->set_handshake_operation(&op); } else { - log_handshake_info(start_time_ns, op->name(), 0, 0, "(thread dead)"); - return; + log_handshake_info(start_time_ns, op.name(), 0, 0, "(thread dead)"); + return false; } bool by_handshaker = false; - while (!op->is_completed()) { - by_handshaker = target->handshake_try_process(op); + while (!op.is_completed()) { + by_handshaker = target->handshake_try_process(&op); // Check for pending handshakes to avoid possible deadlocks where our // target is trying to handshake us. if (SafepointMechanism::should_block(self)) { ThreadBlockInVM tbivm(self); } } - DEBUG_ONLY(op->check_state();) - log_handshake_info(start_time_ns, op->name(), 1, by_handshaker ? 1 : 0); + DEBUG_ONLY(op.check_state();) + log_handshake_info(start_time_ns, op.name(), 1, by_handshaker ? 1 : 0); + + return op.executed(); } HandshakeState::HandshakeState() : _operation(NULL), _operation_direct(NULL), _handshake_turn_sem(1), _processing_sem(1), _thread_in_process_handshake(false) { @@ -345,18 +343,21 @@ if (!_processing_sem.trywait()) { _processing_sem.wait_with_safepoint_check(_thread); } - bool is_direct_handshake = false; - HandshakeOperation* op = Atomic::load_acquire(&_operation); - if (op == NULL) { - op = Atomic::load_acquire(&_operation_direct); - is_direct_handshake = true; - } - if (op != NULL) { + if (has_operation()) { HandleMark hm(_thread); CautiouslyPreserveExceptionMark pem(_thread); - // Disarm before execute the operation - clear_handshake(is_direct_handshake); - op->do_handshake(_thread); + HandshakeOperation * op = _operation; + if (op != NULL) { + // Disarm before execute the operation + clear_handshake(false); + op->do_handshake(_thread); + } + op = _operation_direct; + if (op != NULL) { + // Disarm before execute the operation + clear_handshake(true); + op->do_handshake(_thread); + } } _processing_sem.signal(); } while (has_operation()); @@ -393,7 +394,7 @@ if (!_processing_sem.trywait()) { return false; } - if ((!is_direct && _operation != NULL) || (is_direct && _operation_direct != NULL)){ + if (has_specific_operation(is_direct)){ return true; } _processing_sem.signal(); @@ -403,7 +404,7 @@ bool HandshakeState::try_process(HandshakeOperation* op) { bool is_direct = op->is_direct(); - if ((!is_direct && _operation == NULL) || (is_direct && _operation_direct == NULL)){ + if (!has_specific_operation(is_direct)){ // JT has already cleared its handshake return false; } --- old/src/hotspot/share/runtime/handshake.hpp 2020-01-15 18:32:12.806834552 +0000 +++ new/src/hotspot/share/runtime/handshake.hpp 2020-01-15 18:32:11.958814397 +0000 @@ -37,7 +37,10 @@ // while that thread is in a safepoint safe state. The callback is executed // either by the thread itself or by the VM thread while keeping the thread // in a blocked state. A handshake can be performed with a single -// JavaThread as well. +// JavaThread as well. In that case the callback is executed either by the +// thread itself or, depending on wether the operation is a direct handshake +// or not, by the JavaThread that requested the handshake or the VMThread +// respectively. class HandshakeClosure : public ThreadClosure { const char* const _name; public: @@ -49,17 +52,17 @@ }; class Handshake : public AllStatic { - static void direct_handshake(JavaThread* target, HandshakeOperation *op); public: // Execution of handshake operation static void execute(HandshakeClosure* hs_cl); - static bool execute(HandshakeClosure* hs_cl, JavaThread* target, bool is_direct_handshake = false); + static bool execute(HandshakeClosure* hs_cl, JavaThread* target); + static bool execute_direct(HandshakeClosure* hs_cl, JavaThread* target); }; // The HandshakeState keep tracks of an ongoing handshake for one JavaThread. -// VM thread and JavaThread are serialized with the semaphore making sure -// the operation is only done by either VM thread on behalf of the JavaThread -// or the JavaThread itself. +// VMThread/Handshaker and JavaThread are serialized with the semaphore making +// sure the operation is only done by either VMThread/Handshaker on behalf of +// the JavaThread or the JavaThread itself. class HandshakeState { JavaThread *_thread; HandshakeOperation* volatile _operation; @@ -83,6 +86,9 @@ void set_operation(HandshakeOperation* op); bool has_operation() const { return _operation != NULL || _operation_direct != NULL; } + bool has_specific_operation(bool is_direct) const { + return is_direct ? _operation_direct != NULL : _operation != NULL; + } void process_by_self() { if (!_thread_in_process_handshake) { --- old/test/hotspot/jtreg/runtime/handshake/HandshakeDirectTest.java 2020-01-15 18:32:13.824858748 +0000 +++ new/test/hotspot/jtreg/runtime/handshake/HandshakeDirectTest.java 2020-01-15 18:32:13.000839163 +0000 @@ -24,7 +24,7 @@ /* * @test HandshakeDirectTest - * @summary This test tries to stress direct handshakes between threads while suspending them. + * @summary This test tries to stress direct handshakes between threads. * @library /testlibrary /test/lib * @build HandshakeDirectTest * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+SafepointALot -XX:BiasedLockingDecayTime=100000000 -XX:BiasedLockingBulkRebiasThreshold=1000000 -XX:BiasedLockingBulkRevokeThreshold=1000000 HandshakeDirectTest @@ -32,17 +32,17 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.Semaphore; import java.io.*; public class HandshakeDirectTest implements Runnable { static final int WORKING_THREADS = 32; static final int DIRECT_HANDSHAKES_MARK = 50000; - static Thread[] _working_threads = new Thread[WORKING_THREADS]; - static java.util.concurrent.Semaphore[] _handshake_sem = new java.util.concurrent.Semaphore[WORKING_THREADS]; - static Object[] _locks = new Object[WORKING_THREADS]; - static boolean[] _is_biased = new boolean[WORKING_THREADS]; - static Thread _suspendresume_thread = new Thread(); - static AtomicInteger _handshake_count = new AtomicInteger(0); + static Thread[] workingThreads = new Thread[WORKING_THREADS]; + static Semaphore[] handshakeSem = new Semaphore[WORKING_THREADS]; + static Object[] locks = new Object[WORKING_THREADS]; + static boolean[] isBiased = new boolean[WORKING_THREADS]; + static AtomicInteger handshakeCount = new AtomicInteger(0); @Override public void run() { @@ -50,12 +50,12 @@ while (true) { try { - if (_is_biased[me] == false) { - _handshake_sem[me].acquire(); - synchronized(_locks[me]) { - _is_biased[me] = true; + if (!isBiased[me]) { + handshakeSem[me].acquire(); + synchronized(locks[me]) { + isBiased[me] = true; } - _handshake_sem[me].release(); + handshakeSem[me].release(); } // Handshake directly some other worker @@ -63,21 +63,22 @@ if (handshakee == me) { handshakee = handshakee != 0 ? handshakee - 1 : handshakee + 1; } - _handshake_sem[handshakee].acquire(); - if (_is_biased[handshakee]) { + handshakeSem[handshakee].acquire(); + if (isBiased[handshakee]) { // Revoke biased lock - synchronized(_locks[handshakee]) { - _handshake_count.incrementAndGet(); + synchronized(locks[handshakee]) { + handshakeCount.incrementAndGet(); } // Create new lock to be biased - _locks[handshakee] = new Object(); - _is_biased[handshakee] = false; + locks[handshakee] = new Object(); + isBiased[handshakee] = false; } - _handshake_sem[handshakee].release(); - if (_handshake_count.get() >= DIRECT_HANDSHAKES_MARK) { + handshakeSem[handshakee].release(); + if (handshakeCount.get() >= DIRECT_HANDSHAKES_MARK) { break; } } catch(InterruptedException ie) { + throw new Error("Unexpected interrupt"); } } } @@ -87,42 +88,22 @@ // Initialize semaphores for (int i = 0; i < WORKING_THREADS; i++) { - _handshake_sem[i] = new java.util.concurrent.Semaphore(1); + handshakeSem[i] = new Semaphore(1); } // Initialize locks for (int i = 0; i < WORKING_THREADS; i++) { - _locks[i] = new Object(); + locks[i] = new Object(); } // Fire-up working threads. for (int i = 0; i < WORKING_THREADS; i++) { - _working_threads[i] = new Thread(test, Integer.toString(i)); - _working_threads[i].setDaemon(true); - _working_threads[i].start(); + workingThreads[i] = new Thread(test, Integer.toString(i)); + workingThreads[i].setDaemon(true); + workingThreads[i].start(); } - // Fire-up suspend-resume thread - Thread _suspendresume_thread = new Thread() { - @Override - public void run() { - while (true) { - int i = ThreadLocalRandom.current().nextInt(0, WORKING_THREADS-1); - _working_threads[i].suspend(); - try { - Thread.sleep(1); // sleep for 1 ms - } catch(InterruptedException ie) { - } - _working_threads[i].resume(); - } - } - }; - _suspendresume_thread.setDaemon(true); - _suspendresume_thread.start(); - // Wait until the desired number of direct handshakes is reached - while (_handshake_count.get() < DIRECT_HANDSHAKES_MARK) { - Thread.sleep(10); // sleep for 10ms - } + workingThreads[0].join(); } }