8183128: Update RefineCardTableEntryClosure

Improve names and placement of RefineCardTableEntryClosure

Reviewed-by: ehelin, sjohanss

hotspot/src/share/vm/gc/g1/concurrentG1Refine.cpp

@@ -169,8 +169,7 @@ static size_t calc_init_red_zone(size_t green, size_t yellow) {
   return MIN2(yellow + size, max_red_zone);
 }
 
-ConcurrentG1Refine* ConcurrentG1Refine::create(CardTableEntryClosure* refine_closure,
+ConcurrentG1Refine* ConcurrentG1Refine::create(jint* ecode) {
-                                               jint* ecode) {
   size_t min_yellow_zone_size = calc_min_yellow_zone_size();
   size_t green_zone = calc_init_green_zone();
   size_t yellow_zone = calc_init_yellow_zone(green_zone, min_yellow_zone_size);
@@ -209,7 +208,6 @@ ConcurrentG1Refine* ConcurrentG1Refine::create(CardTableEntryClosure* refine_clo
     ConcurrentG1RefineThread* t =
       new ConcurrentG1RefineThread(cg1r,
                                    next,
-                                   refine_closure,
                                    worker_id_offset,
                                    i,
                                    activation_level(thresholds),

hotspot/src/share/vm/gc/g1/concurrentG1Refine.hpp

@@ -80,7 +80,7 @@ class ConcurrentG1Refine: public CHeapObj<mtGC> {
 
   // Returns ConcurrentG1Refine instance if succeeded to create/initialize ConcurrentG1Refine and ConcurrentG1RefineThread.
   // Otherwise, returns NULL with error code.
-  static ConcurrentG1Refine* create(CardTableEntryClosure* refine_closure, jint* ecode);
+  static ConcurrentG1Refine* create(jint* ecode);
 
   void stop();
 

hotspot/src/share/vm/gc/g1/concurrentG1RefineThread.cpp

@@ -26,6 +26,7 @@
 #include "gc/g1/concurrentG1Refine.hpp"
 #include "gc/g1/concurrentG1RefineThread.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
+#include "gc/g1/g1RemSet.hpp"
 #include "gc/g1/suspendibleThreadSet.hpp"
 #include "logging/log.hpp"
 #include "memory/resourceArea.hpp"
@@ -34,11 +35,9 @@
 
 ConcurrentG1RefineThread::
 ConcurrentG1RefineThread(ConcurrentG1Refine* cg1r, ConcurrentG1RefineThread *next,
-                         CardTableEntryClosure* refine_closure,
                          uint worker_id_offset, uint worker_id,
                          size_t activate, size_t deactivate) :
   ConcurrentGCThread(),
-  _refine_closure(refine_closure),
   _worker_id_offset(worker_id_offset),
   _worker_id(worker_id),
   _active(false),
@@ -145,10 +144,7 @@ void ConcurrentG1RefineThread::run_service() {
         }
 
         // Process the next buffer, if there are enough left.
-        if (!dcqs.apply_closure_to_completed_buffer(_refine_closure,
+        if (!dcqs.refine_completed_buffer_concurrently(_worker_id + _worker_id_offset, _deactivation_threshold)) {
-                                                    _worker_id + _worker_id_offset,
-                                                    _deactivation_threshold,
-                                                    false /* during_pause */)) {
           break; // Deactivate, number of buffers fell below threshold.
         }
         ++buffers_processed;

hotspot/src/share/vm/gc/g1/concurrentG1RefineThread.hpp

@@ -25,6 +25,7 @@
 #ifndef SHARE_VM_GC_G1_CONCURRENTG1REFINETHREAD_HPP
 #define SHARE_VM_GC_G1_CONCURRENTG1REFINETHREAD_HPP
 
+#include "gc/g1/dirtyCardQueue.hpp"
 #include "gc/shared/concurrentGCThread.hpp"
 
 // Forward Decl.
@@ -50,9 +51,6 @@ class ConcurrentG1RefineThread: public ConcurrentGCThread {
   Monitor* _monitor;
   ConcurrentG1Refine* _cg1r;
 
-  // The closure applied to completed log buffers.
-  CardTableEntryClosure* _refine_closure;
-
   // This thread's activation/deactivation thresholds
   size_t _activation_threshold;
   size_t _deactivation_threshold;
@@ -72,7 +70,6 @@ class ConcurrentG1RefineThread: public ConcurrentGCThread {
 public:
   // Constructor
   ConcurrentG1RefineThread(ConcurrentG1Refine* cg1r, ConcurrentG1RefineThread* next,
-                           CardTableEntryClosure* refine_closure,
                            uint worker_id_offset, uint worker_id,
                            size_t activate, size_t deactivate);
 

hotspot/src/share/vm/gc/g1/dirtyCardQueue.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "gc/g1/dirtyCardQueue.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
+#include "gc/g1/g1RemSet.hpp"
 #include "gc/g1/heapRegionRemSet.hpp"
 #include "gc/shared/workgroup.hpp"
 #include "runtime/atomic.hpp"
@@ -32,6 +33,24 @@
 #include "runtime/safepoint.hpp"
 #include "runtime/thread.inline.hpp"
 
+// Closure used for updating remembered sets and recording references that
+// point into the collection set while the mutator is running.
+// Assumed to be only executed concurrently with the mutator. Yields via
+// SuspendibleThreadSet after every card.
+class G1RefineCardConcurrentlyClosure: public CardTableEntryClosure {
+public:
+  bool do_card_ptr(jbyte* card_ptr, uint worker_i) {
+    G1CollectedHeap::heap()->g1_rem_set()->refine_card_concurrently(card_ptr, worker_i);
+
+    if (SuspendibleThreadSet::should_yield()) {
+      // Caller will actually yield.
+      return false;
+    }
+    // Otherwise, we finished successfully; return true.
+    return true;
+  }
+};
+
 // Represents a set of free small integer ids.
 class FreeIdSet : public CHeapObj<mtGC> {
   enum {
@@ -112,7 +131,6 @@ DirtyCardQueue::~DirtyCardQueue() {
 
 DirtyCardQueueSet::DirtyCardQueueSet(bool notify_when_complete) :
   PtrQueueSet(notify_when_complete),
-  _mut_process_closure(NULL),
   _shared_dirty_card_queue(this, true /* permanent */),
   _free_ids(NULL),
   _processed_buffers_mut(0), _processed_buffers_rs_thread(0)
@@ -125,15 +143,13 @@ uint DirtyCardQueueSet::num_par_ids() {
   return (uint)os::initial_active_processor_count();
 }
 
-void DirtyCardQueueSet::initialize(CardTableEntryClosure* cl,
+void DirtyCardQueueSet::initialize(Monitor* cbl_mon,
-                                   Monitor* cbl_mon,
                                    Mutex* fl_lock,
                                    int process_completed_threshold,
                                    int max_completed_queue,
                                    Mutex* lock,
                                    DirtyCardQueueSet* fl_owner,
                                    bool init_free_ids) {
-  _mut_process_closure = cl;
   PtrQueueSet::initialize(cbl_mon,
                           fl_lock,
                           process_completed_threshold,
@@ -192,7 +208,8 @@ bool DirtyCardQueueSet::mut_process_buffer(BufferNode* node) {
   guarantee(_free_ids != NULL, "must be");
 
   uint worker_i = _free_ids->claim_par_id(); // temporarily claim an id
-  bool result = apply_closure_to_buffer(_mut_process_closure, node, true, worker_i);
+  G1RefineCardConcurrentlyClosure cl;
+  bool result = apply_closure_to_buffer(&cl, node, true, worker_i);
   _free_ids->release_par_id(worker_i); // release the id
 
   if (result) {
@@ -226,6 +243,16 @@ BufferNode* DirtyCardQueueSet::get_completed_buffer(size_t stop_at) {
   return nd;
 }
 
+bool DirtyCardQueueSet::refine_completed_buffer_concurrently(uint worker_i, size_t stop_at) {
+  G1RefineCardConcurrentlyClosure cl;
+  return apply_closure_to_completed_buffer(&cl, worker_i, stop_at, false);
+}
+
+bool DirtyCardQueueSet::apply_closure_during_gc(CardTableEntryClosure* cl, uint worker_i) {
+  assert_at_safepoint(false);
+  return apply_closure_to_completed_buffer(cl, worker_i, 0, true);
+}
+
 bool DirtyCardQueueSet::apply_closure_to_completed_buffer(CardTableEntryClosure* cl,
                                                           uint worker_i,
                                                           size_t stop_at,

hotspot/src/share/vm/gc/g1/dirtyCardQueue.hpp

@@ -68,9 +68,6 @@ public:
 
 
 class DirtyCardQueueSet: public PtrQueueSet {
-  // The closure used in mut_process_buffer().
-  CardTableEntryClosure* _mut_process_closure;
-
   DirtyCardQueue _shared_dirty_card_queue;
 
   // Apply the closure to the elements of "node" from it's index to
@@ -85,6 +82,23 @@ class DirtyCardQueueSet: public PtrQueueSet {
                                bool consume,
                                uint worker_i = 0);
 
+  // If there are more than stop_at completed buffers, pop one, apply
+  // the specified closure to its active elements, and return true.
+  // Otherwise return false.
+  //
+  // A completely processed buffer is freed.  However, if a closure
+  // invocation returns false, processing is stopped and the partially
+  // processed buffer (with its index updated to exclude the processed
+  // elements, e.g. up to the element for which the closure returned
+  // false) is returned to the completed buffer set.
+  //
+  // If during_pause is true, stop_at must be zero, and the closure
+  // must never return false.
+  bool apply_closure_to_completed_buffer(CardTableEntryClosure* cl,
+                                         uint worker_i,
+                                         size_t stop_at,
+                                         bool during_pause);
+
   bool mut_process_buffer(BufferNode* node);
 
   // Protected by the _cbl_mon.
@@ -103,8 +117,7 @@ class DirtyCardQueueSet: public PtrQueueSet {
 public:
   DirtyCardQueueSet(bool notify_when_complete = true);
 
-  void initialize(CardTableEntryClosure* cl,
+  void initialize(Monitor* cbl_mon,
-                  Monitor* cbl_mon,
                   Mutex* fl_lock,
                   int process_completed_threshold,
                   int max_completed_queue,
@@ -118,22 +131,13 @@ public:
 
   static void handle_zero_index_for_thread(JavaThread* t);
 
-  // If there are more than stop_at completed buffers, pop one, apply
+  // Apply G1RefineCardConcurrentlyClosure to completed buffers until there are stop_at
-  // the specified closure to its active elements, and return true.
+  // completed buffers remaining.
-  // Otherwise return false.
+  bool refine_completed_buffer_concurrently(uint worker_i, size_t stop_at);
-  //
+
-  // A completely processed buffer is freed.  However, if a closure
+  // Apply the given closure to all completed buffers. The given closure's do_card_ptr
-  // invocation returns false, processing is stopped and the partially
+  // must never return false. Must only be called during GC.
-  // processed buffer (with its index updated to exclude the processed
+  bool apply_closure_during_gc(CardTableEntryClosure* cl, uint worker_i);
-  // elements, e.g. up to the element for which the closure returned
-  // false) is returned to the completed buffer set.
-  //
-  // If during_pause is true, stop_at must be zero, and the closure
-  // must never return false.
-  bool apply_closure_to_completed_buffer(CardTableEntryClosure* cl,
-                                         uint worker_i,
-                                         size_t stop_at,
-                                         bool during_pause);
 
   BufferNode* get_completed_buffer(size_t stop_at);
 

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

@@ -94,28 +94,6 @@ size_t G1CollectedHeap::_humongous_object_threshold_in_words = 0;
 // apply to TLAB allocation, which is not part of this interface: it
 // is done by clients of this interface.)
 
-// Local to this file.
-
-class RefineCardTableEntryClosure: public CardTableEntryClosure {
-  bool _concurrent;
-public:
-  RefineCardTableEntryClosure() : _concurrent(true) { }
-
-  bool do_card_ptr(jbyte* card_ptr, uint worker_i) {
-    G1CollectedHeap::heap()->g1_rem_set()->refine_card_concurrently(card_ptr, worker_i);
-
-    if (_concurrent && SuspendibleThreadSet::should_yield()) {
-      // Caller will actually yield.
-      return false;
-    }
-    // Otherwise, we finished successfully; return true.
-    return true;
-  }
-
-  void set_concurrent(bool b) { _concurrent = b; }
-};
-
-
 class RedirtyLoggedCardTableEntryClosure : public CardTableEntryClosure {
  private:
   size_t _num_dirtied;
@@ -1701,7 +1679,6 @@ G1CollectedHeap::G1CollectedHeap(G1CollectorPolicy* collector_policy) :
   _g1_rem_set(NULL),
   _cg1r(NULL),
   _g1mm(NULL),
-  _refine_cte_cl(NULL),
   _preserved_marks_set(true /* in_c_heap */),
   _secondary_free_list("Secondary Free List", new SecondaryFreeRegionListMtSafeChecker()),
   _old_set("Old Set", false /* humongous */, new OldRegionSetMtSafeChecker()),
@@ -1780,10 +1757,8 @@ G1RegionToSpaceMapper* G1CollectedHeap::create_aux_memory_mapper(const char* des
 }
 
 jint G1CollectedHeap::initialize_concurrent_refinement() {
-  _refine_cte_cl = new RefineCardTableEntryClosure();
-
   jint ecode = JNI_OK;
-  _cg1r = ConcurrentG1Refine::create(_refine_cte_cl, &ecode);
+  _cg1r = ConcurrentG1Refine::create(&ecode);
   return ecode;
 }
 
@@ -1940,8 +1915,7 @@ jint G1CollectedHeap::initialize() {
     return ecode;
   }
 
-  JavaThread::dirty_card_queue_set().initialize(_refine_cte_cl,
+  JavaThread::dirty_card_queue_set().initialize(DirtyCardQ_CBL_mon,
-                                                DirtyCardQ_CBL_mon,
                                                 DirtyCardQ_FL_lock,
                                                 (int)concurrent_g1_refine()->yellow_zone(),
                                                 (int)concurrent_g1_refine()->red_zone(),
@@ -1949,8 +1923,7 @@ jint G1CollectedHeap::initialize() {
                                                 NULL,  // fl_owner
                                                 true); // init_free_ids
 
-  dirty_card_queue_set().initialize(NULL, // Should never be called by the Java code
+  dirty_card_queue_set().initialize(DirtyCardQ_CBL_mon,
-                                    DirtyCardQ_CBL_mon,
                                     DirtyCardQ_FL_lock,
                                     -1, // never trigger processing
                                     -1, // no limit on length
@@ -2127,7 +2100,7 @@ void G1CollectedHeap::iterate_hcc_closure(CardTableEntryClosure* cl, uint worker
 void G1CollectedHeap::iterate_dirty_card_closure(CardTableEntryClosure* cl, uint worker_i) {
   DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
   size_t n_completed_buffers = 0;
-  while (dcqs.apply_closure_to_completed_buffer(cl, worker_i, 0, true)) {
+  while (dcqs.apply_closure_during_gc(cl, worker_i)) {
     n_completed_buffers++;
   }
   g1_policy()->phase_times()->record_thread_work_item(G1GCPhaseTimes::UpdateRS, worker_i, n_completed_buffers);
@@ -5281,10 +5254,6 @@ void G1CollectedHeap::rebuild_region_sets(bool free_list_only) {
          used_unlocked(), recalculate_used());
 }
 
-void G1CollectedHeap::set_refine_cte_cl_concurrency(bool concurrent) {
-  _refine_cte_cl->set_concurrent(concurrent);
-}
-
 bool G1CollectedHeap::is_in_closed_subset(const void* p) const {
   HeapRegion* hr = heap_region_containing(p);
   return hr->is_in(p);

hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp

@@ -109,8 +109,6 @@ public:
   bool do_object_b(oop p);
 };
 
-class RefineCardTableEntryClosure;
-
 class G1RegionMappingChangedListener : public G1MappingChangedListener {
  private:
   void reset_from_card_cache(uint start_idx, size_t num_regions);
@@ -781,9 +779,6 @@ protected:
   // concurrently after the collection.
   DirtyCardQueueSet _dirty_card_queue_set;
 
-  // The closure used to refine a single card.
-  RefineCardTableEntryClosure* _refine_cte_cl;
-
   // After a collection pause, convert the regions in the collection set into free
   // regions.
   void free_collection_set(G1CollectionSet* collection_set, EvacuationInfo& evacuation_info, const size_t* surviving_young_words);
@@ -940,8 +935,6 @@ protected:
 
 public:
 
-  void set_refine_cte_cl_concurrency(bool concurrent);
-
   RefToScanQueue *task_queue(uint i) const;
 
   uint num_task_queues() const;

hotspot/src/share/vm/gc/g1/g1RemSet.cpp

@@ -36,6 +36,7 @@
 #include "gc/g1/heapRegion.inline.hpp"
 #include "gc/g1/heapRegionManager.inline.hpp"
 #include "gc/g1/heapRegionRemSet.hpp"
+#include "gc/g1/suspendibleThreadSet.hpp"
 #include "gc/shared/gcTraceTime.inline.hpp"
 #include "memory/iterator.hpp"
 #include "memory/resourceArea.hpp"
@@ -292,8 +293,7 @@ G1RemSet::G1RemSet(G1CollectedHeap* g1,
 {
   // Initialize the card queue set used to hold cards containing
   // references into the collection set.
-  _into_cset_dirty_card_queue_set.initialize(NULL, // Should never be called by the Java code
+  _into_cset_dirty_card_queue_set.initialize(DirtyCardQ_CBL_mon,
-                                             DirtyCardQ_CBL_mon,
                                              DirtyCardQ_FL_lock,
                                              -1, // never trigger processing
                                              -1, // no limit on length
@@ -519,7 +519,6 @@ void G1RemSet::oops_into_collection_set_do(G1ParScanThreadState* pss,
 }
 
 void G1RemSet::prepare_for_oops_into_collection_set_do() {
-  _g1->set_refine_cte_cl_concurrency(false);
   DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
   dcqs.concatenate_logs();
 
@@ -528,8 +527,6 @@ void G1RemSet::prepare_for_oops_into_collection_set_do() {
 
 void G1RemSet::cleanup_after_oops_into_collection_set_do() {
   G1GCPhaseTimes* phase_times = _g1->g1_policy()->phase_times();
-  // Cleanup after copy
-  _g1->set_refine_cte_cl_concurrency(true);
 
   // Set all cards back to clean.
   double start = os::elapsedTime();
@@ -803,7 +800,7 @@ void G1RemSet::print_summary_info() {
   Log(gc, remset, exit) log;
   if (log.is_trace()) {
     log.trace(" Cumulative RS summary");
-    G1RemSetSummary current;
+    G1RemSetSummary current(this);
     ResourceMark rm;
     current.print_on(log.trace_stream());
   }