8138862: Remove some unused code and subclasses in gcTaskManager.hpp/cpp

Reviewed-by: mgerdin, jwilhelm

hotspot/src/share/vm/gc/parallel/gcTaskManager.cpp

@@ -48,8 +48,8 @@ const char* GCTask::Kind::to_string(kind value) {
   case ordinary_task:
     result = "ordinary task";
     break;
-  case barrier_task:
+  case wait_for_barrier_task:
-    result = "barrier task";
+    result = "wait for barrier task";
     break;
   case noop_task:
     result = "noop task";
@@ -378,16 +378,7 @@ SynchronizedGCTaskQueue::~SynchronizedGCTaskQueue() {
 GCTaskManager::GCTaskManager(uint workers) :
   _workers(workers),
   _active_workers(0),
-  _idle_workers(0),
+  _idle_workers(0) {
-  _ndc(NULL) {
-  initialize();
-}
-
-GCTaskManager::GCTaskManager(uint workers, NotifyDoneClosure* ndc) :
-  _workers(workers),
-  _active_workers(0),
-  _idle_workers(0),
-  _ndc(ndc) {
   initialize();
 }
 
@@ -437,7 +428,6 @@ void GCTaskManager::initialize() {
   }
   reset_delivered_tasks();
   reset_completed_tasks();
-  reset_noop_tasks();
   reset_barriers();
   reset_emptied_queue();
   for (uint s = 0; s < workers(); s += 1) {
@@ -671,7 +661,6 @@ GCTask* GCTaskManager::get_task(uint which) {
     // Just hand back a Noop task,
     // in case someone wanted us to release resources, or whatever.
     result = noop_task();
-    increment_noop_tasks();
   }
   assert(result != NULL, "shouldn't have null task");
   if (TraceGCTaskManager) {
@@ -706,11 +695,6 @@ void GCTaskManager::note_completion(uint which) {
     if (TraceGCTaskManager) {
       tty->print_cr("    GCTaskManager::note_completion(%u) done", which);
     }
-    // Notify client that we are done.
-    NotifyDoneClosure* ndc = notify_done_closure();
-    if (ndc != NULL) {
-      ndc->notify(this);
-    }
   }
   if (TraceGCTaskManager) {
     tty->print_cr("    GCTaskManager::note_completion(%u) (%s)->notify_all",
@@ -751,7 +735,7 @@ uint GCTaskManager::decrement_busy_workers() {
 }
 
 void GCTaskManager::release_all_resources() {
-  // If you want this to be done atomically, do it in a BarrierGCTask.
+  // If you want this to be done atomically, do it in a WaitForBarrierGCTask.
   for (uint i = 0; i < workers(); i += 1) {
     set_resource_flag(i, true);
   }
@@ -813,22 +797,15 @@ void GCTaskManager::set_resource_flag(uint which, bool value) {
 // NoopGCTask
 //
 
-NoopGCTask* NoopGCTask::create() {
-  NoopGCTask* result = new NoopGCTask(false);
-  return result;
-}
-
 NoopGCTask* NoopGCTask::create_on_c_heap() {
-  NoopGCTask* result = new(ResourceObj::C_HEAP, mtGC) NoopGCTask(true);
+  NoopGCTask* result = new(ResourceObj::C_HEAP, mtGC) NoopGCTask();
   return result;
 }
 
 void NoopGCTask::destroy(NoopGCTask* that) {
   if (that != NULL) {
     that->destruct();
-    if (that->is_c_heap_obj()) {
+    FreeHeap(that);
-      FreeHeap(that);
-    }
   }
 }
 
@@ -908,74 +885,6 @@ void IdleGCTask::destruct() {
   // Nothing else to do.
 }
 
-//
-// BarrierGCTask
-//
-
-void BarrierGCTask::do_it(GCTaskManager* manager, uint which) {
-  // Wait for this to be the only busy worker.
-  // ??? I thought of having a StackObj class
-  //     whose constructor would grab the lock and come to the barrier,
-  //     and whose destructor would release the lock,
-  //     but that seems like too much mechanism for two lines of code.
-  MutexLockerEx ml(manager->lock(), Mutex::_no_safepoint_check_flag);
-  do_it_internal(manager, which);
-  // Release manager->lock().
-}
-
-void BarrierGCTask::do_it_internal(GCTaskManager* manager, uint which) {
-  // Wait for this to be the only busy worker.
-  assert(manager->monitor()->owned_by_self(), "don't own the lock");
-  assert(manager->is_blocked(), "manager isn't blocked");
-  while (manager->busy_workers() > 1) {
-    if (TraceGCTaskManager) {
-      tty->print_cr("BarrierGCTask::do_it(%u) waiting on %u workers",
-                    which, manager->busy_workers());
-    }
-    manager->monitor()->wait(Mutex::_no_safepoint_check_flag, 0);
-  }
-}
-
-void BarrierGCTask::destruct() {
-  this->GCTask::destruct();
-  // Nothing else to do.
-}
-
-//
-// ReleasingBarrierGCTask
-//
-
-void ReleasingBarrierGCTask::do_it(GCTaskManager* manager, uint which) {
-  MutexLockerEx ml(manager->lock(), Mutex::_no_safepoint_check_flag);
-  do_it_internal(manager, which);
-  manager->release_all_resources();
-  // Release manager->lock().
-}
-
-void ReleasingBarrierGCTask::destruct() {
-  this->BarrierGCTask::destruct();
-  // Nothing else to do.
-}
-
-//
-// NotifyingBarrierGCTask
-//
-
-void NotifyingBarrierGCTask::do_it(GCTaskManager* manager, uint which) {
-  MutexLockerEx ml(manager->lock(), Mutex::_no_safepoint_check_flag);
-  do_it_internal(manager, which);
-  NotifyDoneClosure* ndc = notify_done_closure();
-  if (ndc != NULL) {
-    ndc->notify(manager);
-  }
-  // Release manager->lock().
-}
-
-void NotifyingBarrierGCTask::destruct() {
-  this->BarrierGCTask::destruct();
-  // Nothing else to do.
-}
-
 //
 // WaitForBarrierGCTask
 //
@@ -991,6 +900,7 @@ WaitForBarrierGCTask* WaitForBarrierGCTask::create_on_c_heap() {
 }
 
 WaitForBarrierGCTask::WaitForBarrierGCTask(bool on_c_heap) :
+  GCTask(GCTask::Kind::wait_for_barrier_task),
   _is_c_heap_obj(on_c_heap) {
   _monitor = MonitorSupply::reserve();
   set_should_wait(true);
@@ -1028,7 +938,7 @@ void WaitForBarrierGCTask::destruct() {
                   "  monitor: " INTPTR_FORMAT,
                   p2i(this), p2i(monitor()));
   }
-  this->BarrierGCTask::destruct();
+  this->GCTask::destruct();
   // Clean up that should be in the destructor,
   // except that ResourceMarks don't call destructors.
    if (monitor() != NULL) {
@@ -1037,6 +947,19 @@ void WaitForBarrierGCTask::destruct() {
   _monitor = (Monitor*) 0xDEAD000F;
 }
 
+void WaitForBarrierGCTask::do_it_internal(GCTaskManager* manager, uint which) {
+  // Wait for this to be the only busy worker.
+  assert(manager->monitor()->owned_by_self(), "don't own the lock");
+  assert(manager->is_blocked(), "manager isn't blocked");
+  while (manager->busy_workers() > 1) {
+    if (TraceGCTaskManager) {
+      tty->print_cr("WaitForBarrierGCTask::do_it(%u) waiting on %u workers",
+                    which, manager->busy_workers());
+    }
+    manager->monitor()->wait(Mutex::_no_safepoint_check_flag, 0);
+  }
+}
+
 void WaitForBarrierGCTask::do_it(GCTaskManager* manager, uint which) {
   if (TraceGCTaskManager) {
     tty->print_cr("[" INTPTR_FORMAT "]"

hotspot/src/share/vm/gc/parallel/gcTaskManager.hpp

@@ -38,12 +38,8 @@ class GCTask;
 class GCTaskQueue;
 class SynchronizedGCTaskQueue;
 class GCTaskManager;
-class NotifyDoneClosure;
 // Some useful subclasses of GCTask.  You can also make up your own.
 class NoopGCTask;
-class BarrierGCTask;
-class ReleasingBarrierGCTask;
-class NotifyingBarrierGCTask;
 class WaitForBarrierGCTask;
 class IdleGCTask;
 // A free list of Monitor*'s.
@@ -64,7 +60,7 @@ public:
     enum kind {
       unknown_task,
       ordinary_task,
-      barrier_task,
+      wait_for_barrier_task,
       noop_task,
       idle_task
     };
@@ -105,7 +101,7 @@ public:
     return kind()==Kind::ordinary_task;
   }
   bool is_barrier_task() const {
-    return kind()==Kind::barrier_task;
+    return kind()==Kind::wait_for_barrier_task;
   }
   bool is_noop_task() const {
     return kind()==Kind::noop_task;
@@ -276,23 +272,6 @@ protected:
   ~SynchronizedGCTaskQueue();
 };
 
-// This is an abstract base class for getting notifications
-// when a GCTaskManager is done.
-class NotifyDoneClosure : public CHeapObj<mtGC> {
-public:
-  // The notification callback method.
-  virtual void notify(GCTaskManager* manager) = 0;
-protected:
-  // Constructor.
-  NotifyDoneClosure() {
-    // Nothing to do.
-  }
-  // Virtual destructor because virtual methods.
-  virtual ~NotifyDoneClosure() {
-    // Nothing to do.
-  }
-};
-
 // Dynamic number of GC threads
 //
 //  GC threads wait in get_task() for work (i.e., a task) to perform.
@@ -365,7 +344,6 @@ class GCTaskManager : public CHeapObj<mtGC> {
  friend class IdleGCTask;
 private:
   // Instance state.
-  NotifyDoneClosure*        _ndc;               // Notify on completion.
   const uint                _workers;           // Number of workers.
   Monitor*                  _monitor;           // Notification of changes.
   SynchronizedGCTaskQueue*  _queue;             // Queue of tasks.
@@ -379,7 +357,6 @@ private:
   uint                      _barriers;          // Count of barrier tasks.
   uint                      _emptied_queue;     // Times we emptied the queue.
   NoopGCTask*               _noop_task;         // The NoopGCTask instance.
-  uint                      _noop_tasks;        // Count of noop tasks.
   WaitForBarrierGCTask*     _idle_inactive_task;// Task for inactive workers
   volatile uint             _idle_workers;      // Number of idled workers
 public:
@@ -387,9 +364,6 @@ public:
   static GCTaskManager* create(uint workers) {
     return new GCTaskManager(workers);
   }
-  static GCTaskManager* create(uint workers, NotifyDoneClosure* ndc) {
-    return new GCTaskManager(workers, ndc);
-  }
   static void destroy(GCTaskManager* that) {
     if (that != NULL) {
       delete that;
@@ -452,8 +426,6 @@ protected:
   // Constructors.  Clients use factory, but there might be subclasses.
   //     Create a GCTaskManager with the appropriate number of workers.
   GCTaskManager(uint workers);
-  //     Create a GCTaskManager that calls back when there's no more work.
-  GCTaskManager(uint workers, NotifyDoneClosure* ndc);
   //     Make virtual if necessary.
   ~GCTaskManager();
   // Accessors.
@@ -469,9 +441,6 @@ protected:
   // Sets the number of threads that will be used in a collection
   void set_active_gang();
 
-  NotifyDoneClosure* notify_done_closure() const {
-    return _ndc;
-  }
   SynchronizedGCTaskQueue* queue() const {
     return _queue;
   }
@@ -540,17 +509,6 @@ protected:
   void reset_emptied_queue() {
     _emptied_queue = 0;
   }
-  //     Count of the number of noop tasks we've handed out,
-  //     e.g., to handle resource release requests.
-  uint noop_tasks() const {
-    return _noop_tasks;
-  }
-  void increment_noop_tasks() {
-    _noop_tasks += 1;
-  }
-  void reset_noop_tasks() {
-    _noop_tasks = 0;
-  }
   void increment_idle_workers() {
     _idle_workers++;
   }
@@ -575,11 +533,8 @@ protected:
 // A noop task that does nothing,
 // except take us around the GCTaskThread loop.
 class NoopGCTask : public GCTask {
-private:
-  const bool _is_c_heap_obj;            // Is this a CHeapObj?
 public:
   // Factory create and destroy methods.
-  static NoopGCTask* create();
   static NoopGCTask* create_on_c_heap();
   static void destroy(NoopGCTask* that);
 
@@ -590,113 +545,16 @@ public:
   }
 protected:
   // Constructor.
-  NoopGCTask(bool on_c_heap) :
+  NoopGCTask() :
-    GCTask(GCTask::Kind::noop_task),
+    GCTask(GCTask::Kind::noop_task) { }
-    _is_c_heap_obj(on_c_heap) {
-    // Nothing to do.
-  }
-  // Destructor-like method.
-  void destruct();
-  // Accessors.
-  bool is_c_heap_obj() const {
-    return _is_c_heap_obj;
-  }
-};
-
-// A BarrierGCTask blocks other tasks from starting,
-// and waits until it is the only task running.
-class BarrierGCTask : public GCTask {
-public:
-  // Factory create and destroy methods.
-  static BarrierGCTask* create() {
-    return new BarrierGCTask();
-  }
-  static void destroy(BarrierGCTask* that) {
-    if (that != NULL) {
-      that->destruct();
-      delete that;
-    }
-  }
-  // Methods from GCTask.
-  void do_it(GCTaskManager* manager, uint which);
-protected:
-  // Constructor.  Clients use factory, but there might be subclasses.
-  BarrierGCTask() :
-    GCTask(GCTask::Kind::barrier_task) {
-    // Nothing to do.
-  }
-  // Destructor-like method.
-  void destruct();
-
-  virtual char* name() { return (char *)"barrier task"; }
-  // Methods.
-  //     Wait for this to be the only task running.
-  void do_it_internal(GCTaskManager* manager, uint which);
-};
-
-// A ReleasingBarrierGCTask is a BarrierGCTask
-// that tells all the tasks to release their resource areas.
-class ReleasingBarrierGCTask : public BarrierGCTask {
-public:
-  // Factory create and destroy methods.
-  static ReleasingBarrierGCTask* create() {
-    return new ReleasingBarrierGCTask();
-  }
-  static void destroy(ReleasingBarrierGCTask* that) {
-    if (that != NULL) {
-      that->destruct();
-      delete that;
-    }
-  }
-  // Methods from GCTask.
-  void do_it(GCTaskManager* manager, uint which);
-protected:
-  // Constructor.  Clients use factory, but there might be subclasses.
-  ReleasingBarrierGCTask() :
-    BarrierGCTask() {
-    // Nothing to do.
-  }
   // Destructor-like method.
   void destruct();
 };
 
-// A NotifyingBarrierGCTask is a BarrierGCTask
+// A WaitForBarrierGCTask is a GCTask
-// that calls a notification method when it is the only task running.
-class NotifyingBarrierGCTask : public BarrierGCTask {
-private:
-  // Instance state.
-  NotifyDoneClosure* _ndc;              // The callback object.
-public:
-  // Factory create and destroy methods.
-  static NotifyingBarrierGCTask* create(NotifyDoneClosure* ndc) {
-    return new NotifyingBarrierGCTask(ndc);
-  }
-  static void destroy(NotifyingBarrierGCTask* that) {
-    if (that != NULL) {
-      that->destruct();
-      delete that;
-    }
-  }
-  // Methods from GCTask.
-  void do_it(GCTaskManager* manager, uint which);
-protected:
-  // Constructor.  Clients use factory, but there might be subclasses.
-  NotifyingBarrierGCTask(NotifyDoneClosure* ndc) :
-    BarrierGCTask(),
-    _ndc(ndc) {
-    assert(notify_done_closure() != NULL, "can't notify on NULL");
-  }
-  // Destructor-like method.
-  void destruct();
-  // Accessor.
-  NotifyDoneClosure* notify_done_closure() const { return _ndc; }
-};
-
-// A WaitForBarrierGCTask is a BarrierGCTask
 // with a method you can call to wait until
 // the BarrierGCTask is done.
-// This may cover many of the uses of NotifyingBarrierGCTasks.
+class WaitForBarrierGCTask : public GCTask {
-class WaitForBarrierGCTask : public BarrierGCTask {
   friend class GCTaskManager;
   friend class IdleGCTask;
 private:
@@ -722,6 +580,11 @@ protected:
   WaitForBarrierGCTask(bool on_c_heap);
   // Destructor-like method.
   void destruct();
+
+  // Methods.
+  //     Wait for this to be the only task running.
+  void do_it_internal(GCTaskManager* manager, uint which);
+
   // Accessors.
   Monitor* monitor() const {
     return _monitor;