# HG changeset patch # User kbarrett # Date 1535486694 14400 # Node ID 35a6956f424349756bc86bdc636a14f80d85b270 # Parent 2b004d807187234158806526dddc2c480eb59368 8209976: Improve iteration over non-JavaThreads Summary: Add NonJavaThread and move NamedThread iteration to new class. Reviewed-by: eosterlund, coleenp, rkennke diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp --- a/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp Tue Aug 28 16:04:54 2018 -0400 @@ -308,7 +308,7 @@ _added_native(0) { } -class JfrThreadSampler : public Thread { +class JfrThreadSampler : public NonJavaThread { friend class JfrThreadSampling; private: Semaphore _sample; diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/runtime/mutexLocker.cpp --- a/src/hotspot/share/runtime/mutexLocker.cpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/runtime/mutexLocker.cpp Tue Aug 28 16:04:54 2018 -0400 @@ -76,7 +76,7 @@ Monitor* Safepoint_lock = NULL; Monitor* SerializePage_lock = NULL; Monitor* Threads_lock = NULL; -Mutex* NamedThreadsList_lock = NULL; +Mutex* NonJavaThreadsList_lock = NULL; Monitor* CGC_lock = NULL; Monitor* STS_lock = NULL; Monitor* FullGCCount_lock = NULL; @@ -257,7 +257,7 @@ def(Safepoint_lock , PaddedMonitor, safepoint, true, Monitor::_safepoint_check_sometimes); // locks SnippetCache_lock/Threads_lock def(Threads_lock , PaddedMonitor, barrier, true, Monitor::_safepoint_check_sometimes); - def(NamedThreadsList_lock , PaddedMutex, leaf, true, Monitor::_safepoint_check_never); + def(NonJavaThreadsList_lock , PaddedMutex, leaf, true, Monitor::_safepoint_check_never); def(VMOperationQueue_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_sometimes); // VM_thread allowed to block on these def(VMOperationRequest_lock , PaddedMonitor, nonleaf, true, Monitor::_safepoint_check_sometimes); diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/runtime/mutexLocker.hpp --- a/src/hotspot/share/runtime/mutexLocker.hpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/runtime/mutexLocker.hpp Tue Aug 28 16:04:54 2018 -0400 @@ -72,7 +72,7 @@ extern Monitor* Safepoint_lock; // a lock used by the safepoint abstraction extern Monitor* Threads_lock; // a lock on the Threads table of active Java threads // (also used by Safepoints too to block threads creation/destruction) -extern Mutex* NamedThreadsList_lock; // a lock on the NamedThreads list +extern Mutex* NonJavaThreadsList_lock; // a lock on the NonJavaThreads list extern Monitor* CGC_lock; // used for coordination between // fore- & background GC threads. extern Monitor* STS_lock; // used for joining/leaving SuspendibleThreadSet. diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/runtime/thread.cpp --- a/src/hotspot/share/runtime/thread.cpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/runtime/thread.cpp Tue Aug 28 16:04:54 2018 -0400 @@ -86,6 +86,7 @@ #include "runtime/prefetch.inline.hpp" #include "runtime/safepoint.hpp" #include "runtime/safepointMechanism.inline.hpp" +#include "runtime/safepointVerifiers.hpp" #include "runtime/sharedRuntime.hpp" #include "runtime/statSampler.hpp" #include "runtime/stubRoutines.hpp" @@ -168,13 +169,6 @@ // Current thread is maintained as a thread-local variable THREAD_LOCAL_DECL Thread* Thread::_thr_current = NULL; #endif -// Class hierarchy -// - Thread -// - VMThread -// - WatcherThread -// - ConcurrentMarkSweepThread -// - JavaThread -// - CompilerThread // ======= Thread ======== // Support for forcing alignment of thread objects for biased locking @@ -1207,61 +1201,63 @@ THREAD); } -// List of all NamedThreads and safe iteration over that list. - -class NamedThread::List { +// List of all NonJavaThreads and safe iteration over that list. + +class NonJavaThread::List { public: - NamedThread* volatile _head; + NonJavaThread* volatile _head; SingleWriterSynchronizer _protect; List() : _head(NULL), _protect() {} }; -NamedThread::List NamedThread::_the_list; - -NamedThread::Iterator::Iterator() : +NonJavaThread::List NonJavaThread::_the_list; + +NonJavaThread::Iterator::Iterator() : _protect_enter(_the_list._protect.enter()), _current(OrderAccess::load_acquire(&_the_list._head)) {} -NamedThread::Iterator::~Iterator() { +NonJavaThread::Iterator::~Iterator() { _the_list._protect.exit(_protect_enter); } -void NamedThread::Iterator::step() { +void NonJavaThread::Iterator::step() { assert(!end(), "precondition"); - _current = OrderAccess::load_acquire(&_current->_next_named_thread); + _current = OrderAccess::load_acquire(&_current->_next); +} + +NonJavaThread::NonJavaThread() : Thread(), _next(NULL) { + // Add this thread to _the_list. + MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag); + _next = _the_list._head; + OrderAccess::release_store(&_the_list._head, this); +} + +NonJavaThread::~NonJavaThread() { + // Remove this thread from _the_list. + MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag); + NonJavaThread* volatile* p = &_the_list._head; + for (NonJavaThread* t = *p; t != NULL; p = &t->_next, t = *p) { + if (t == this) { + *p = this->_next; + // Wait for any in-progress iterators. + _the_list._protect.synchronize(); + break; + } + } } // NamedThread -- non-JavaThread subclasses with multiple // uniquely named instances should derive from this. NamedThread::NamedThread() : - Thread(), + NonJavaThread(), _name(NULL), _processed_thread(NULL), - _gc_id(GCId::undefined()), - _next_named_thread(NULL) -{ - // Add this thread to _the_list. - MutexLockerEx lock(NamedThreadsList_lock, Mutex::_no_safepoint_check_flag); - _next_named_thread = _the_list._head; - OrderAccess::release_store(&_the_list._head, this); -} + _gc_id(GCId::undefined()) +{} NamedThread::~NamedThread() { - // Remove this thread from _the_list. - { - MutexLockerEx lock(NamedThreadsList_lock, Mutex::_no_safepoint_check_flag); - NamedThread* volatile* p = &_the_list._head; - for (NamedThread* t = *p; t != NULL; p = &t->_next_named_thread, t = *p) { - if (t == this) { - *p = this->_next_named_thread; - // Wait for any in-progress iterators. - _the_list._protect.synchronize(); - break; - } - } - } if (_name != NULL) { FREE_C_HEAP_ARRAY(char, _name); _name = NULL; @@ -1299,7 +1295,7 @@ bool WatcherThread::_startable = false; volatile bool WatcherThread::_should_terminate = false; -WatcherThread::WatcherThread() : Thread() { +WatcherThread::WatcherThread() : NonJavaThread() { assert(watcher_thread() == NULL, "we can only allocate one WatcherThread"); if (os::create_thread(this, os::watcher_thread)) { _watcher_thread = this; @@ -3430,35 +3426,12 @@ // All JavaThreads #define ALL_JAVA_THREADS(X) DO_JAVA_THREADS(ThreadsSMRSupport::get_java_thread_list(), X) -// All non-JavaThreads (i.e., every non-JavaThread in the system). +// All NonJavaThreads (i.e., every non-JavaThread in the system). void Threads::non_java_threads_do(ThreadClosure* tc) { - // Someday we could have a table or list of all non-JavaThreads. - // For now, just manually iterate through them. - tc->do_thread(VMThread::vm_thread()); - if (Universe::heap() != NULL) { - Universe::heap()->gc_threads_do(tc); - } - WatcherThread *wt = WatcherThread::watcher_thread(); - // Strictly speaking, the following NULL check isn't sufficient to make sure - // the data for WatcherThread is still valid upon being examined. However, - // considering that WatchThread terminates when the VM is on the way to - // exit at safepoint, the chance of the above is extremely small. The right - // way to prevent termination of WatcherThread would be to acquire - // Terminator_lock, but we can't do that without violating the lock rank - // checking in some cases. - if (wt != NULL) { - tc->do_thread(wt); - } - -#if INCLUDE_JFR - Thread* sampler_thread = Jfr::sampler_thread(); - if (sampler_thread != NULL) { - tc->do_thread(sampler_thread); - } - -#endif - - // If CompilerThreads ever become non-JavaThreads, add them here + NoSafepointVerifier nsv(!SafepointSynchronize::is_at_safepoint(), false); + for (NonJavaThread::Iterator njti; !njti.end(); njti.step()) { + tc->do_thread(njti.current()); + } } // All JavaThreads diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/runtime/thread.hpp --- a/src/hotspot/share/runtime/thread.hpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/runtime/thread.hpp Tue Aug 28 16:04:54 2018 -0400 @@ -93,16 +93,21 @@ // Class hierarchy // - Thread -// - NamedThread -// - VMThread -// - ConcurrentGCThread -// - WorkerThread -// - GangWorker -// - GCTaskThread // - JavaThread // - various subclasses eg CompilerThread, ServiceThread -// - WatcherThread -// - JfrSamplerThread +// - NonJavaThread +// - NamedThread +// - VMThread +// - ConcurrentGCThread +// - WorkerThread +// - GangWorker +// - GCTaskThread +// - WatcherThread +// - JfrThreadSampler +// +// All Thread subclasses must be either JavaThread or NonJavaThread. +// This means !t->is_Java_thread() iff t is a NonJavaThread, or t is +// a partially constructed/destroyed Thread. class Thread: public ThreadShadow { friend class VMStructs; @@ -380,7 +385,7 @@ // Constructor Thread(); - virtual ~Thread(); + virtual ~Thread() = 0; // Thread is abstract. // Manage Thread::current() void initialize_thread_current(); @@ -764,9 +769,47 @@ return NULL; } +class NonJavaThread: public Thread { + friend class VMStructs; + + NonJavaThread* volatile _next; + + class List; + static List _the_list; + + public: + NonJavaThread(); + ~NonJavaThread(); + + class Iterator; +}; + +// Provides iteration over the list of NonJavaThreads. Because list +// management occurs in the NonJavaThread constructor and destructor, +// entries in the list may not be fully constructed instances of a +// derived class. Threads created after an iterator is constructed +// will not be visited by the iterator. The scope of an iterator is a +// critical section; there must be no safepoint checks in that scope. +class NonJavaThread::Iterator : public StackObj { + uint _protect_enter; + NonJavaThread* _current; + + // Noncopyable. + Iterator(const Iterator&); + Iterator& operator=(const Iterator&); + +public: + Iterator(); + ~Iterator(); + + bool end() const { return _current == NULL; } + NonJavaThread* current() const { return _current; } + void step(); +}; + // Name support for threads. non-JavaThread subclasses with multiple // uniquely named instances should derive from this. -class NamedThread: public Thread { +class NamedThread: public NonJavaThread { friend class VMStructs; enum { max_name_len = 64 @@ -776,10 +819,6 @@ // log JavaThread being processed by oops_do JavaThread* _processed_thread; uint _gc_id; // The current GC id when a thread takes part in GC - NamedThread* volatile _next_named_thread; - - class List; - static List _the_list; public: NamedThread(); @@ -795,31 +834,6 @@ void set_gc_id(uint gc_id) { _gc_id = gc_id; } uint gc_id() { return _gc_id; } - - class Iterator; -}; - -// Provides iteration over the list of NamedThreads. Because list -// management occurs in the NamedThread constructor and destructor, -// entries in the list may not be fully constructed instances of a -// derived class. Threads created after an iterator is constructed -// will not be visited by the iterator. The scope of an iterator is a -// critical section; there must be no safepoint checks in that scope. -class NamedThread::Iterator : public StackObj { - uint _protect_enter; - NamedThread* _current; - - // Noncopyable. - Iterator(const Iterator&); - Iterator& operator=(const Iterator&); - -public: - Iterator(); - ~Iterator(); - - bool end() const { return _current == NULL; } - NamedThread* current() const { return _current; } - void step(); }; // Worker threads are named and have an id of an assigned work. @@ -840,7 +854,7 @@ }; // A single WatcherThread is used for simulating timer interrupts. -class WatcherThread: public Thread { +class WatcherThread: public NonJavaThread { friend class VMStructs; public: virtual void run(); diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/runtime/vmStructs.cpp --- a/src/hotspot/share/runtime/vmStructs.cpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/runtime/vmStructs.cpp Tue Aug 28 16:04:54 2018 -0400 @@ -1355,14 +1355,15 @@ \ declare_toplevel_type(Threads) \ declare_toplevel_type(ThreadShadow) \ - declare_type(Thread, ThreadShadow) \ - declare_type(NamedThread, Thread) \ - declare_type(WatcherThread, Thread) \ - declare_type(JavaThread, Thread) \ - declare_type(JvmtiAgentThread, JavaThread) \ - declare_type(ServiceThread, JavaThread) \ - declare_type(CompilerThread, JavaThread) \ - declare_type(CodeCacheSweeperThread, JavaThread) \ + declare_type(Thread, ThreadShadow) \ + declare_type(NonJavaThread, Thread) \ + declare_type(NamedThread, NonJavaThread) \ + declare_type(WatcherThread, NonJavaThread) \ + declare_type(JavaThread, Thread) \ + declare_type(JvmtiAgentThread, JavaThread) \ + declare_type(ServiceThread, JavaThread) \ + declare_type(CompilerThread, JavaThread) \ + declare_type(CodeCacheSweeperThread, JavaThread) \ declare_toplevel_type(OSThread) \ declare_toplevel_type(JavaFrameAnchor) \ \ diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/utilities/globalCounter.cpp --- a/src/hotspot/share/utilities/globalCounter.cpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/utilities/globalCounter.cpp Tue Aug 28 16:04:54 2018 -0400 @@ -71,7 +71,7 @@ for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thread = jtiwh.next(); ) { ctc.do_thread(thread); } - for (NamedThread::Iterator nti; !nti.end(); nti.step()) { - ctc.do_thread(nti.current()); + for (NonJavaThread::Iterator njti; !njti.end(); njti.step()) { + ctc.do_thread(njti.current()); } } diff -r 2b004d807187 -r 35a6956f4243 src/hotspot/share/utilities/globalCounter.inline.hpp --- a/src/hotspot/share/utilities/globalCounter.inline.hpp Tue Aug 28 14:45:34 2018 -0400 +++ b/src/hotspot/share/utilities/globalCounter.inline.hpp Tue Aug 28 16:04:54 2018 -0400 @@ -31,7 +31,6 @@ inline void GlobalCounter::critical_section_begin(Thread *thread) { assert(thread == Thread::current(), "must be current thread"); - assert(thread->is_Named_thread() || thread->is_Java_thread(), "must be NamedThread or JavaThread"); assert((*thread->get_rcu_counter() & COUNTER_ACTIVE) == 0x0, "nested critical sections, not supported yet"); uintx gbl_cnt = OrderAccess::load_acquire(&_global_counter._counter); OrderAccess::release_store_fence(thread->get_rcu_counter(), gbl_cnt | COUNTER_ACTIVE); @@ -39,7 +38,6 @@ inline void GlobalCounter::critical_section_end(Thread *thread) { assert(thread == Thread::current(), "must be current thread"); - assert(thread->is_Named_thread() || thread->is_Java_thread(), "must be NamedThread or JavaThread"); assert((*thread->get_rcu_counter() & COUNTER_ACTIVE) == COUNTER_ACTIVE, "must be in critical section"); // Mainly for debugging we set it to 'now'. uintx gbl_cnt = OrderAccess::load_acquire(&_global_counter._counter);