8151126: Clean up duplicate code for clearing the mark bitmaps
authortschatzl
Wed, 16 Mar 2016 12:21:18 +0100
changeset 37122 5318124fe3a8
parent 37114 f9047a59010f
child 37123 bf4f064a10a8
8151126: Clean up duplicate code for clearing the mark bitmaps Summary: Merge the code to clear the mark bitmap concurrently and in the STW pause. Reviewed-by: jmasa, tbenson
hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp
hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp
--- a/hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp	Wed Mar 16 05:46:41 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/concurrentMarkThread.cpp	Wed Mar 16 12:21:18 2016 +0100
@@ -266,7 +266,7 @@
       // bitmap then, as it has been done during mark abort.
       if (!cm()->has_aborted()) {
         GCConcPhaseTimer(_cm, "Concurrent Bitmap Clearing");
-        _cm->clearNextBitmap();
+        _cm->cleanup_for_next_mark();
       } else {
         assert(!G1VerifyBitmaps || _cm->nextMarkBitmapIsClear(), "Next mark bitmap must be clear");
       }
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Mar 16 05:46:41 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Mar 16 12:21:18 2016 +0100
@@ -1423,7 +1423,7 @@
       // the full GC has compacted objects and updated TAMS but not updated
       // the prev bitmap.
       if (G1VerifyBitmaps) {
-        ((G1CMBitMap*) concurrent_mark()->prevMarkBitMap())->clearAll();
+        _cm->clear_prev_bitmap(workers());
       }
       _verifier->check_bitmaps("Full GC End");
 
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Wed Mar 16 05:46:41 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Wed Mar 16 12:21:18 2016 +0100
@@ -120,74 +120,10 @@
   }
   // We need to clear the bitmap on commit, removing any existing information.
   MemRegion mr(G1CollectedHeap::heap()->bottom_addr_for_region(start_region), num_regions * HeapRegion::GrainWords);
-  _bm->clearRange(mr);
+  _bm->clear_range(mr);
 }
 
-// Closure used for clearing the given mark bitmap.
-class ClearBitmapHRClosure : public HeapRegionClosure {
- private:
-  G1ConcurrentMark* _cm;
-  G1CMBitMap* _bitmap;
-  bool _may_yield;      // The closure may yield during iteration. If yielded, abort the iteration.
- public:
-  ClearBitmapHRClosure(G1ConcurrentMark* cm, G1CMBitMap* bitmap, bool may_yield) : HeapRegionClosure(), _cm(cm), _bitmap(bitmap), _may_yield(may_yield) {
-    assert(!may_yield || cm != NULL, "CM must be non-NULL if this closure is expected to yield.");
-  }
-
-  virtual bool doHeapRegion(HeapRegion* r) {
-    size_t const chunk_size_in_words = M / HeapWordSize;
-
-    HeapWord* cur = r->bottom();
-    HeapWord* const end = r->end();
-
-    while (cur < end) {
-      MemRegion mr(cur, MIN2(cur + chunk_size_in_words, end));
-      _bitmap->clearRange(mr);
-
-      cur += chunk_size_in_words;
-
-      // Abort iteration if after yielding the marking has been aborted.
-      if (_may_yield && _cm->do_yield_check() && _cm->has_aborted()) {
-        return true;
-      }
-      // Repeat the asserts from before the start of the closure. We will do them
-      // as asserts here to minimize their overhead on the product. However, we
-      // will have them as guarantees at the beginning / end of the bitmap
-      // clearing to get some checking in the product.
-      assert(!_may_yield || _cm->cmThread()->during_cycle(), "invariant");
-      assert(!_may_yield || !G1CollectedHeap::heap()->collector_state()->mark_in_progress(), "invariant");
-    }
-
-    return false;
-  }
-};
-
-class ParClearNextMarkBitmapTask : public AbstractGangTask {
-  ClearBitmapHRClosure* _cl;
-  HeapRegionClaimer     _hrclaimer;
-  bool                  _suspendible; // If the task is suspendible, workers must join the STS.
-
-public:
-  ParClearNextMarkBitmapTask(ClearBitmapHRClosure *cl, uint n_workers, bool suspendible) :
-      _cl(cl), _suspendible(suspendible), AbstractGangTask("Parallel Clear Bitmap Task"), _hrclaimer(n_workers) {}
-
-  void work(uint worker_id) {
-    SuspendibleThreadSetJoiner sts_join(_suspendible);
-    G1CollectedHeap::heap()->heap_region_par_iterate(_cl, worker_id, &_hrclaimer, true);
-  }
-};
-
-void G1CMBitMap::clearAll() {
-  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-  ClearBitmapHRClosure cl(NULL, this, false /* may_yield */);
-  uint n_workers = g1h->workers()->active_workers();
-  ParClearNextMarkBitmapTask task(&cl, n_workers, false);
-  g1h->workers()->run_task(&task);
-  guarantee(cl.complete(), "Must have completed iteration.");
-  return;
-}
-
-void G1CMBitMap::clearRange(MemRegion mr) {
+void G1CMBitMap::clear_range(MemRegion mr) {
   mr.intersection(MemRegion(_bmStartWord, _bmWordSize));
   assert(!mr.is_empty(), "unexpected empty region");
   // convert address range into offset range
@@ -697,9 +633,76 @@
   ShouldNotReachHere();
 }
 
-void G1ConcurrentMark::clearNextBitmap() {
-  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-
+class G1ClearBitMapTask : public AbstractGangTask {
+  // Heap region closure used for clearing the given mark bitmap.
+  class G1ClearBitmapHRClosure : public HeapRegionClosure {
+  private:
+    G1CMBitMap* _bitmap;
+    G1ConcurrentMark* _cm;
+  public:
+    G1ClearBitmapHRClosure(G1CMBitMap* bitmap, G1ConcurrentMark* cm) : HeapRegionClosure(), _cm(cm), _bitmap(bitmap) {
+    }
+
+    virtual bool doHeapRegion(HeapRegion* r) {
+      size_t const chunk_size_in_words = M / HeapWordSize;
+
+      HeapWord* cur = r->bottom();
+      HeapWord* const end = r->end();
+
+      while (cur < end) {
+        MemRegion mr(cur, MIN2(cur + chunk_size_in_words, end));
+        _bitmap->clear_range(mr);
+
+        cur += chunk_size_in_words;
+
+        // Abort iteration if after yielding the marking has been aborted.
+        if (_cm != NULL && _cm->do_yield_check() && _cm->has_aborted()) {
+          return true;
+        }
+        // Repeat the asserts from before the start of the closure. We will do them
+        // as asserts here to minimize their overhead on the product. However, we
+        // will have them as guarantees at the beginning / end of the bitmap
+        // clearing to get some checking in the product.
+        assert(_cm == NULL || _cm->cmThread()->during_cycle(), "invariant");
+        assert(_cm == NULL || !G1CollectedHeap::heap()->collector_state()->mark_in_progress(), "invariant");
+      }
+      assert(cur == end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index());
+
+      return false;
+    }
+  };
+
+  G1ClearBitmapHRClosure _cl;
+  HeapRegionClaimer _hr_claimer;
+  bool _suspendible; // If the task is suspendible, workers must join the STS.
+
+public:
+  G1ClearBitMapTask(G1CMBitMap* bitmap, G1ConcurrentMark* cm, uint n_workers, bool suspendible) :
+    AbstractGangTask("Parallel Clear Bitmap Task"),
+    _cl(bitmap, suspendible ? cm : NULL),
+    _hr_claimer(n_workers),
+    _suspendible(suspendible)
+  { }
+
+  void work(uint worker_id) {
+    SuspendibleThreadSetJoiner sts_join(_suspendible);
+    G1CollectedHeap::heap()->heap_region_par_iterate(&_cl, worker_id, &_hr_claimer, true);
+  }
+
+  bool is_complete() {
+    return _cl.complete();
+  }
+};
+
+void G1ConcurrentMark::clear_bitmap(G1CMBitMap* bitmap, WorkGang* workers, bool may_yield) {
+  assert(may_yield || SafepointSynchronize::is_at_safepoint(), "Non-yielding bitmap clear only allowed at safepoint.");
+
+  G1ClearBitMapTask task(bitmap, this, workers->active_workers(), may_yield);
+  workers->run_task(&task);
+  guarantee(!may_yield || task.is_complete(), "Must have completed iteration when not yielding.");
+}
+
+void G1ConcurrentMark::cleanup_for_next_mark() {
   // Make sure that the concurrent mark thread looks to still be in
   // the current cycle.
   guarantee(cmThread()->during_cycle(), "invariant");
@@ -708,21 +711,24 @@
   // marking bitmap and getting it ready for the next cycle. During
   // this time no other cycle can start. So, let's make sure that this
   // is the case.
-  guarantee(!g1h->collector_state()->mark_in_progress(), "invariant");
-
-  ClearBitmapHRClosure cl(this, _nextMarkBitMap, true /* may_yield */);
-  ParClearNextMarkBitmapTask task(&cl, parallel_marking_threads(), true);
-  _parallel_workers->run_task(&task);
+  guarantee(!_g1h->collector_state()->mark_in_progress(), "invariant");
+
+  clear_bitmap(_nextMarkBitMap, _parallel_workers, true);
 
   // Clear the liveness counting data. If the marking has been aborted, the abort()
   // call already did that.
-  if (cl.complete()) {
+  if (!has_aborted()) {
     clear_all_count_data();
   }
 
   // Repeat the asserts from above.
   guarantee(cmThread()->during_cycle(), "invariant");
-  guarantee(!g1h->collector_state()->mark_in_progress(), "invariant");
+  guarantee(!_g1h->collector_state()->mark_in_progress(), "invariant");
+}
+
+void G1ConcurrentMark::clear_prev_bitmap(WorkGang* workers) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Should only clear the entire prev bitmap at a safepoint.");
+  clear_bitmap((G1CMBitMap*)_prevMarkBitMap, workers, false);
 }
 
 class CheckBitmapClearHRClosure : public HeapRegionClosure {
@@ -2307,7 +2313,7 @@
 void G1ConcurrentMark::clearRangePrevBitmap(MemRegion mr) {
   // Note we are overriding the read-only view of the prev map here, via
   // the cast.
-  ((G1CMBitMap*)_prevMarkBitMap)->clearRange(mr);
+  ((G1CMBitMap*)_prevMarkBitMap)->clear_range(mr);
 }
 
 HeapRegion*
@@ -2604,7 +2610,7 @@
 
   // Clear all marks in the next bitmap for the next marking cycle. This will allow us to skip the next
   // concurrent bitmap clearing.
-  _nextMarkBitMap->clearAll();
+  clear_bitmap(_nextMarkBitMap, _g1h->workers(), false);
 
   // Note we cannot clear the previous marking bitmap here
   // since VerifyDuringGC verifies the objects marked during
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Wed Mar 16 05:46:41 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Wed Mar 16 12:21:18 2016 +0100
@@ -139,10 +139,7 @@
   inline void clear(HeapWord* addr);
   inline bool parMark(HeapWord* addr);
 
-  void clearRange(MemRegion mr);
-
-  // Clear the whole mark bitmap.
-  void clearAll();
+  void clear_range(MemRegion mr);
 };
 
 // Represents a marking stack used by ConcurrentMarking in the G1 collector.
@@ -497,6 +494,9 @@
   // end_timer, true to end gc timer after ending concurrent phase.
   void register_concurrent_phase_end_common(bool end_timer);
 
+  // Clear the given bitmap in parallel using the given WorkGang. If may_yield is
+  // true, periodically insert checks to see if this method should exit prematurely.
+  void clear_bitmap(G1CMBitMap* bitmap, WorkGang* workers, bool may_yield);
 public:
   // Manipulation of the global mark stack.
   // The push and pop operations are used by tasks for transfers
@@ -585,8 +585,13 @@
                        uint worker_id,
                        HeapRegion* hr = NULL);
 
-  // Clear the next marking bitmap (will be called concurrently).
-  void clearNextBitmap();
+  // Prepare internal data structures for the next mark cycle. This includes clearing
+  // the next mark bitmap and some internal data structures. This method is intended
+  // to be called concurrently to the mutator. It will yield to safepoint requests.
+  void cleanup_for_next_mark();
+
+  // Clear the previous marking bitmap during safepoint.
+  void clear_prev_bitmap(WorkGang* workers);
 
   // Return whether the next mark bitmap has no marks set. To be used for assertions
   // only. Will not yield to pause requests.