8210557: G1 next bitmap verification at the end of concurrent mark sometimes fails
authortschatzl
Fri, 21 Sep 2018 15:11:09 +0200
changeset 51835 b177af763b82
parent 51834 5dd9f3ac52a4
child 51836 d62ebdfd8f18
child 56901 d5992bef7aa9
8210557: G1 next bitmap verification at the end of concurrent mark sometimes fails Summary: Removed unnecessary verification that can cause spurious false alarm. Reviewed-by: sjohanss, kbarrett
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
src/hotspot/share/gc/g1/g1ConcurrentMark.hpp
src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Thu Sep 20 13:59:39 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Fri Sep 21 15:11:09 2018 +0200
@@ -1079,9 +1079,10 @@
   // the full GC has compacted objects and updated TAMS but not updated
   // the prev bitmap.
   if (G1VerifyBitmaps) {
-    GCTraceTime(Debug, gc)("Clear Bitmap for Verification");
+    GCTraceTime(Debug, gc)("Clear Prev Bitmap for Verification");
     _cm->clear_prev_bitmap(workers());
   }
+  // This call implicitly verifies that the next bitmap is clear after Full GC.
   _verifier->check_bitmaps("Full GC End");
 
   // At this point there should be no regions in the
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp	Thu Sep 20 13:59:39 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp	Fri Sep 21 15:11:09 2018 +0200
@@ -716,28 +716,6 @@
   clear_bitmap(_prev_mark_bitmap, workers, false);
 }
 
-class CheckBitmapClearHRClosure : public HeapRegionClosure {
-  G1CMBitMap* _bitmap;
- public:
-  CheckBitmapClearHRClosure(G1CMBitMap* bitmap) : _bitmap(bitmap) {
-  }
-
-  virtual bool do_heap_region(HeapRegion* r) {
-    // This closure can be called concurrently to the mutator, so we must make sure
-    // that the result of the getNextMarkedWordAddress() call is compared to the
-    // value passed to it as limit to detect any found bits.
-    // end never changes in G1.
-    HeapWord* end = r->end();
-    return _bitmap->get_next_marked_addr(r->bottom(), end) != end;
-  }
-};
-
-bool G1ConcurrentMark::next_mark_bitmap_is_clear() {
-  CheckBitmapClearHRClosure cl(_next_mark_bitmap);
-  _g1h->heap_region_iterate(&cl);
-  return cl.is_complete();
-}
-
 class NoteStartOfMarkHRClosure : public HeapRegionClosure {
 public:
   bool do_heap_region(HeapRegion* r) {
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp	Thu Sep 20 13:59:39 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp	Fri Sep 21 15:11:09 2018 +0200
@@ -544,10 +544,6 @@
   // 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.
-  bool next_mark_bitmap_is_clear();
-
   // These two methods do the work that needs to be done at the start and end of the
   // initial mark pause.
   void pre_initial_mark();
--- a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp	Thu Sep 20 13:59:39 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp	Fri Sep 21 15:11:09 2018 +0200
@@ -381,8 +381,6 @@
       if (!_cm->has_aborted()) {
         G1ConcPhase p(G1ConcurrentPhase::CLEANUP_FOR_NEXT_MARK, this);
         _cm->cleanup_for_next_mark();
-      } else {
-        assert(!G1VerifyBitmaps || _cm->next_mark_bitmap_is_clear(), "Next mark bitmap must be clear");
       }
     }