8209976: Improve iteration over non-JavaThreads
Summary: Add NonJavaThread and move NamedThread iteration to new class.
Reviewed-by: eosterlund, coleenp, rkennke
--- 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;
--- 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);
--- 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.
--- 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
--- 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();
--- 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) \
\
--- 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());
}
}
--- 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);