8273597: Rectify Thread::is_ConcurrentGC_thread()

Reviewed-by: stefank, coleenp

src/hotspot/share/code/nmethod.cpp

@@ -1193,7 +1193,9 @@ void nmethod::make_unloaded() {
   // recorded in instanceKlasses get flushed.
   // Since this work is being done during a GC, defer deleting dependencies from the
   // InstanceKlass.
-  assert(Universe::heap()->is_gc_active() || Thread::current()->is_ConcurrentGC_thread(),
+  assert(Universe::heap()->is_gc_active() ||
+         Thread::current()->is_ConcurrentGC_thread() ||
+         Thread::current()->is_Worker_thread(),
          "should only be called during gc");
   flush_dependencies(/*delete_immediately*/false);
 
@@ -1233,7 +1235,9 @@ void nmethod::make_unloaded() {
   }
 
   // Make the class unloaded - i.e., change state and notify sweeper
-  assert(SafepointSynchronize::is_at_safepoint() || Thread::current()->is_ConcurrentGC_thread(),
+  assert(SafepointSynchronize::is_at_safepoint() ||
+         Thread::current()->is_ConcurrentGC_thread() ||
+         Thread::current()->is_Worker_thread(),
          "must be at safepoint");
 
   {
@@ -1554,7 +1558,9 @@ oop nmethod::oop_at_phantom(int index) const {
 // notifies instanceKlasses that are reachable
 
 void nmethod::flush_dependencies(bool delete_immediately) {
-  DEBUG_ONLY(bool called_by_gc = Universe::heap()->is_gc_active() || Thread::current()->is_ConcurrentGC_thread();)
+  DEBUG_ONLY(bool called_by_gc = Universe::heap()->is_gc_active() ||
+                                 Thread::current()->is_ConcurrentGC_thread() ||
+                                 Thread::current()->is_Worker_thread();)
   assert(called_by_gc != delete_immediately,
   "delete_immediately is false if and only if we are called during GC");
   if (!has_flushed_dependencies()) {

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -1683,8 +1683,7 @@ jint G1CollectedHeap::initialize() {
     _humongous_reclaim_candidates.initialize(reserved(), granularity);
   }
 
-  _workers = new WorkGang("GC Thread", ParallelGCThreads,
+  _workers = new WorkGang("GC Thread", ParallelGCThreads);
-                          false /* are_ConcurrentGC_threads */);
   if (_workers == NULL) {
     return JNI_ENOMEM;
   }

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

@@ -432,7 +432,7 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
   _num_concurrent_workers = ConcGCThreads;
   _max_concurrent_workers = _num_concurrent_workers;
 
-  _concurrent_workers = new WorkGang("G1 Conc", _max_concurrent_workers, true);
+  _concurrent_workers = new WorkGang("G1 Conc", _max_concurrent_workers);
   _concurrent_workers->initialize_workers();
 
   if (!_global_mark_stack.initialize(MarkStackSize, MarkStackSizeMax)) {
@@ -894,7 +894,6 @@ class G1CMConcurrentMarkingTask : public AbstractGangTask {
 
 public:
   void work(uint worker_id) {
-    assert(Thread::current()->is_ConcurrentGC_thread(), "Not a concurrent GC thread");
     ResourceMark rm;
 
     double start_vtime = os::elapsedVTime();
@@ -979,9 +978,6 @@ public:
     AbstractGangTask("G1 Root Region Scan"), _cm(cm) { }
 
   void work(uint worker_id) {
-    assert(Thread::current()->is_ConcurrentGC_thread(),
-           "this should only be done by a conc GC thread");
-
     G1CMRootMemRegions* root_regions = _cm->root_regions();
     const MemRegion* region = root_regions->claim_next();
     while (region != NULL) {

src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp

@@ -118,9 +118,7 @@ class ParallelScavengeHeap : public CollectedHeap {
     _eden_pool(NULL),
     _survivor_pool(NULL),
     _old_pool(NULL),
-    _workers("GC Thread",
+    _workers("GC Thread", ParallelGCThreads) { }
-             ParallelGCThreads,
-             false /* are_ConcurrentGC_threads */) { }
 
   // For use by VM operations
   enum CollectionType {

src/hotspot/share/gc/shared/concurrentGCThread.hpp

@@ -27,6 +27,7 @@
 
 #include "runtime/nonJavaThread.hpp"
 #include "runtime/thread.hpp"
+#include "utilities/debug.hpp"
 
 class ConcurrentGCThread: public NamedThread {
 private:
@@ -42,6 +43,11 @@ protected:
 public:
   ConcurrentGCThread();
 
+  static ConcurrentGCThread* cast(Thread* t) {
+    assert(t->is_ConcurrentGC_thread(), "incorrect cast to ConcurrentGCThread");
+    return static_cast<ConcurrentGCThread*>(t);
+  }
+
   virtual bool is_ConcurrentGC_thread() const { return true; }
 
   virtual void run();

src/hotspot/share/gc/shared/workgroup.cpp

@@ -116,13 +116,12 @@ public:
 };
 // Definitions of WorkGang methods.
 
-WorkGang::WorkGang(const char* name, uint workers, bool are_ConcurrentGC_threads) :
+WorkGang::WorkGang(const char* name, uint workers) :
     _workers(NULL),
     _total_workers(workers),
     _active_workers(UseDynamicNumberOfGCThreads ? 1U : workers),
     _created_workers(0),
     _name(name),
-    _are_ConcurrentGC_threads(are_ConcurrentGC_threads),
     _dispatcher(new GangTaskDispatcher())
   { }
 

src/hotspot/share/gc/shared/workgroup.hpp

@@ -94,9 +94,6 @@ class WorkGang : public CHeapObj<mtInternal> {
   // Printing support.
   const char* _name;
 
-  // Initialize only instance data.
-  const bool _are_ConcurrentGC_threads;
-
   // To get access to the GangTaskDispatcher instance.
   friend class GangWorker;
   GangTaskDispatcher* const _dispatcher;
@@ -115,15 +112,13 @@ class WorkGang : public CHeapObj<mtInternal> {
   GangWorker* allocate_worker(uint which);
 
  public:
-  WorkGang(const char* name, uint workers, bool are_ConcurrentGC_threads);
+  WorkGang(const char* name, uint workers);
 
   ~WorkGang();
 
   // Initialize workers in the gang.  Return true if initialization succeeded.
   void initialize_workers();
 
-  bool are_ConcurrentGC_threads() const { return _are_ConcurrentGC_threads; }
-
   uint total_workers() const { return _total_workers; }
 
   uint created_workers() const {
@@ -213,9 +208,6 @@ protected:
 public:
   GangWorker(WorkGang* gang, uint id);
 
-  // Predicate for Thread
-  bool is_ConcurrentGC_thread() const override { return gang()->are_ConcurrentGC_threads(); }
-
   // Printing
   const char* type_name() const override { return "GCTaskThread"; }
 };

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -495,8 +495,7 @@ ShenandoahHeap::ShenandoahHeap(ShenandoahCollectorPolicy* policy) :
   BarrierSet::set_barrier_set(new ShenandoahBarrierSet(this));
 
   _max_workers = MAX2(_max_workers, 1U);
-  _workers = new ShenandoahWorkGang("Shenandoah GC Threads", _max_workers,
+  _workers = new ShenandoahWorkGang("Shenandoah GC Threads", _max_workers);
-                            /* are_ConcurrentGC_threads */ true);
   if (_workers == NULL) {
     vm_exit_during_initialization("Failed necessary allocation.");
   } else {
@@ -505,8 +504,7 @@ ShenandoahHeap::ShenandoahHeap(ShenandoahCollectorPolicy* policy) :
 
   if (ParallelGCThreads > 1) {
     _safepoint_workers = new ShenandoahWorkGang("Safepoint Cleanup Thread",
-                                                ParallelGCThreads,
+                                                ParallelGCThreads);
-                 /* are_ConcurrentGC_threads */ false);
     _safepoint_workers->initialize_workers();
   }
 }

src/hotspot/share/gc/shenandoah/shenandoahUtils.cpp

@@ -111,9 +111,7 @@ ShenandoahConcurrentPhase::~ShenandoahConcurrentPhase() {
 
 ShenandoahTimingsTracker::ShenandoahTimingsTracker(ShenandoahPhaseTimings::Phase phase) :
   _timings(ShenandoahHeap::heap()->phase_timings()), _phase(phase) {
-  assert(!Thread::current()->is_Worker_thread() &&
+  assert(Thread::current()->is_VM_thread() || Thread::current()->is_ConcurrentGC_thread(),
-              (Thread::current()->is_VM_thread() ||
-               Thread::current()->is_ConcurrentGC_thread()),
           "Must be set by these threads");
   _parent_phase = _current_phase;
   _current_phase = phase;

src/hotspot/share/gc/shenandoah/shenandoahWorkGroup.hpp

@@ -56,9 +56,8 @@ private:
   bool     _initialize_gclab;
 public:
   ShenandoahWorkGang(const char* name,
-           uint workers,
+           uint workers) :
-           bool are_ConcurrentGC_threads) :
+    WorkGang(name, workers), _initialize_gclab(false) {
-    WorkGang(name, workers, are_ConcurrentGC_threads), _initialize_gclab(false) {
     }
 
   // Create a GC worker and install it into the work gang.

src/hotspot/share/gc/z/zCollectedHeap.cpp

@@ -80,8 +80,8 @@ void ZCollectedHeap::initialize_serviceability() {
 class ZStopConcurrentGCThreadClosure : public ThreadClosure {
 public:
   virtual void do_thread(Thread* thread) {
-    if (thread->is_ConcurrentGC_thread() && !thread->is_Worker_thread()) {
+    if (thread->is_ConcurrentGC_thread()) {
-      static_cast<ConcurrentGCThread*>(thread)->stop();
+      ConcurrentGCThread::cast(thread)->stop();
     }
   }
 };

src/hotspot/share/gc/z/zRuntimeWorkers.cpp

@@ -59,8 +59,7 @@ public:
 
 ZRuntimeWorkers::ZRuntimeWorkers() :
     _workers("RuntimeWorker",
-             ParallelGCThreads,
+             ParallelGCThreads) {
-             false /* are_ConcurrentGC_threads */) {
 
   log_info_p(gc, init)("Runtime Workers: %u", _workers.total_workers());
 

src/hotspot/share/gc/z/zWorkers.cpp

@@ -63,8 +63,7 @@ public:
 
 ZWorkers::ZWorkers() :
     _workers("ZWorker",
-             UseDynamicNumberOfGCThreads ? ConcGCThreads : MAX2(ConcGCThreads, ParallelGCThreads),
+             UseDynamicNumberOfGCThreads ? ConcGCThreads : MAX2(ConcGCThreads, ParallelGCThreads)) {
-             true /* are_ConcurrentGC_threads */) {
 
   if (UseDynamicNumberOfGCThreads) {
     log_info_p(gc, init)("GC Workers: %u (dynamic)", _workers.total_workers());

test/hotspot/gtest/gc/g1/test_g1BatchedGangTask.cpp

@@ -33,7 +33,7 @@ class G1BatchedGangTaskWorkers : AllStatic {
   static WorkGang* _work_gang;
   static WorkGang* work_gang() {
     if (_work_gang == nullptr) {
-      _work_gang = new WorkGang("G1 Small Workers", MaxWorkers, false);
+      _work_gang = new WorkGang("G1 Small Workers", MaxWorkers);
       _work_gang->initialize_workers();
       _work_gang->update_active_workers(MaxWorkers);
     }

test/hotspot/gtest/gc/g1/test_g1CardSet.cpp

@@ -50,7 +50,7 @@ class G1CardSetTest : public ::testing::Test {
   static WorkGang* workers() {
     if (_workers == NULL) {
       _max_workers = os::processor_count();
-      _workers = new WorkGang("G1CardSetTest Work Gang", _max_workers, false);
+      _workers = new WorkGang("G1CardSetTest Work Gang", _max_workers);
       _workers->initialize_workers();
       _workers->update_active_workers(_max_workers);
     }

test/hotspot/gtest/gc/g1/test_stressCommitUncommit.cpp

@@ -35,7 +35,7 @@ class G1MapperWorkers : AllStatic {
   static WorkGang* _work_gang;
   static WorkGang* work_gang() {
     if (_work_gang == NULL) {
-      _work_gang = new WorkGang("G1 Small Workers", MaxWorkers, false);
+      _work_gang = new WorkGang("G1 Small Workers", MaxWorkers);
       _work_gang->initialize_workers();
       _work_gang->update_active_workers(MaxWorkers);
     }

test/hotspot/gtest/gc/shared/test_oopStorage.cpp

@@ -878,9 +878,7 @@ WorkGang* OopStorageTestParIteration::_workers = NULL;
 
 WorkGang* OopStorageTestParIteration::workers() {
   if (_workers == NULL) {
-    _workers = new WorkGang("OopStorageTestParIteration workers",
+    _workers = new WorkGang("OopStorageTestParIteration workers", _max_workers);
-                            _max_workers,
-                            false);
     _workers->initialize_workers();
     _workers->update_active_workers(_max_workers);
   }

test/hotspot/gtest/gc/shared/test_oopStorage_parperf.cpp

@@ -76,9 +76,7 @@ WorkGang* OopStorageParIterPerf::_workers = NULL;
 
 WorkGang* OopStorageParIterPerf::workers() const {
   if (_workers == NULL) {
-    WorkGang* wg = new WorkGang("OopStorageParIterPerf workers",
+    WorkGang* wg = new WorkGang("OopStorageParIterPerf workers", _num_workers);
-                                _num_workers,
-                                false);
     wg->initialize_workers();
     wg->update_active_workers(_num_workers);
     _workers = wg;