changeset 13446:7653b75bfe57

8184346: Clean up G1CMBitmap Summary: Trim down and improve the G1CMBitMap class structure. Reviewed-by: mgerdin, shade, rkennke
author tschatzl
date Fri, 04 Aug 2017 14:15:42 +0200
parents 70ae4e940b3b
children ffaec77df3e5
files src/share/vm/gc/g1/g1CardLiveData.cpp src/share/vm/gc/g1/g1CollectedHeap.cpp src/share/vm/gc/g1/g1CollectedHeap.inline.hpp src/share/vm/gc/g1/g1ConcurrentMark.cpp src/share/vm/gc/g1/g1ConcurrentMark.hpp src/share/vm/gc/g1/g1ConcurrentMark.inline.hpp src/share/vm/gc/g1/g1HeapVerifier.cpp src/share/vm/gc/g1/g1HeapVerifier.hpp src/share/vm/gc/g1/heapRegion.hpp src/share/vm/gc/g1/heapRegion.inline.hpp
diffstat 10 files changed, 155 insertions(+), 222 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc/g1/g1CardLiveData.cpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1CardLiveData.cpp	Fri Aug 04 14:15:42 2017 +0200
@@ -221,7 +221,7 @@
     }
     if (hr->is_humongous()) {
       HeapRegion* start_region = hr->humongous_start_region();
-      if (mark_bitmap->isMarked(start_region->bottom())) {
+      if (mark_bitmap->is_marked(start_region->bottom())) {
         mark_card_bitmap_range(start, hr->top());
         return pointer_delta(hr->top(), start, 1);
       } else {
@@ -236,7 +236,7 @@
            p2i(start), p2i(ntams), p2i(hr->end()));
 
     // Find the first marked object at or after "start".
-    start = mark_bitmap->getNextMarkedWordAddress(start, ntams);
+    start = mark_bitmap->get_next_marked_addr(start, ntams);
     while (start < ntams) {
       oop obj = oop(start);
       size_t obj_size = obj->size();
@@ -250,7 +250,7 @@
       marked_bytes += obj_size * HeapWordSize;
 
       // Find the next marked object after this one.
-      start = mark_bitmap->getNextMarkedWordAddress(obj_end, ntams);
+      start = mark_bitmap->get_next_marked_addr(obj_end, ntams);
     }
 
     return marked_bytes;
--- a/src/share/vm/gc/g1/g1CollectedHeap.cpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1CollectedHeap.cpp	Fri Aug 04 14:15:42 2017 +0200
@@ -4945,7 +4945,7 @@
                                p2i(r->bottom()),
                                r->rem_set()->occupied(),
                                r->rem_set()->strong_code_roots_list_length(),
-                               next_bitmap->isMarked(r->bottom()),
+                               next_bitmap->is_marked(r->bottom()),
                                g1h->is_humongous_reclaim_candidate(region_idx),
                                obj->is_typeArray()
                               );
@@ -4962,13 +4962,13 @@
                              p2i(r->bottom()),
                              r->rem_set()->occupied(),
                              r->rem_set()->strong_code_roots_list_length(),
-                             next_bitmap->isMarked(r->bottom()),
+                             next_bitmap->is_marked(r->bottom()),
                              g1h->is_humongous_reclaim_candidate(region_idx),
                              obj->is_typeArray()
                             );
 
     // Need to clear mark bit of the humongous object if already set.
-    if (next_bitmap->isMarked(r->bottom())) {
+    if (next_bitmap->is_marked(r->bottom())) {
       next_bitmap->clear(r->bottom());
     }
     _humongous_objects_reclaimed++;
--- a/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1CollectedHeap.inline.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -135,7 +135,7 @@
 }
 
 inline bool G1CollectedHeap::isMarkedNext(oop obj) const {
-  return _cm->nextMarkBitMap()->isMarked((HeapWord *)obj);
+  return _cm->nextMarkBitMap()->is_marked((HeapWord*)obj);
 }
 
 inline bool G1CollectedHeap::is_in_cset(oop obj) {
--- a/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Fri Aug 04 14:15:42 2017 +0200
@@ -60,41 +60,7 @@
 #include "utilities/align.hpp"
 #include "utilities/growableArray.hpp"
 
-// Concurrent marking bit map wrapper
-
-G1CMBitMapRO::G1CMBitMapRO(int shifter) :
-  _bm(),
-  _shifter(shifter) {
-  _bmStartWord = 0;
-  _bmWordSize = 0;
-}
-
-HeapWord* G1CMBitMapRO::getNextMarkedWordAddress(const HeapWord* addr,
-                                                 const HeapWord* limit) const {
-  // First we must round addr *up* to a possible object boundary.
-  addr = align_up(addr, HeapWordSize << _shifter);
-  size_t addrOffset = heapWordToOffset(addr);
-  assert(limit != NULL, "limit must not be NULL");
-  size_t limitOffset = heapWordToOffset(limit);
-  size_t nextOffset = _bm.get_next_one_offset(addrOffset, limitOffset);
-  HeapWord* nextAddr = offsetToHeapWord(nextOffset);
-  assert(nextAddr >= addr, "get_next_one postcondition");
-  assert(nextAddr == limit || isMarked(nextAddr),
-         "get_next_one postcondition");
-  return nextAddr;
-}
-
-#ifndef PRODUCT
-bool G1CMBitMapRO::covers(MemRegion heap_rs) const {
-  // assert(_bm.map() == _virtual_space.low(), "map inconsistency");
-  assert(((size_t)_bm.size() * ((size_t)1 << _shifter)) == _bmWordSize,
-         "size inconsistency");
-  return _bmStartWord == (HeapWord*)(heap_rs.start()) &&
-         _bmWordSize  == heap_rs.word_size();
-}
-#endif
-
-void G1CMBitMapRO::print_on_error(outputStream* st, const char* prefix) const {
+void G1CMBitMap::print_on_error(outputStream* st, const char* prefix) const {
   _bm.print_on_error(st, prefix);
 }
 
@@ -107,10 +73,9 @@
 }
 
 void G1CMBitMap::initialize(MemRegion heap, G1RegionToSpaceMapper* storage) {
-  _bmStartWord = heap.start();
-  _bmWordSize = heap.word_size();
-
-  _bm = BitMapView((BitMap::bm_word_t*) storage->reserved().start(), _bmWordSize >> _shifter);
+  _covered = heap;
+
+  _bm = BitMapView((BitMap::bm_word_t*) storage->reserved().start(), _covered.word_size() >> _shifter);
 
   storage->set_mapping_changed_listener(&_listener);
 }
@@ -125,11 +90,13 @@
 }
 
 void G1CMBitMap::clear_range(MemRegion mr) {
-  mr.intersection(MemRegion(_bmStartWord, _bmWordSize));
-  assert(!mr.is_empty(), "unexpected empty region");
+  MemRegion intersection = mr.intersection(_covered);
+  assert(!intersection.is_empty(),
+         "Given range from " PTR_FORMAT " to " PTR_FORMAT " is completely outside the heap",
+         p2i(mr.start()), p2i(mr.end()));
   // convert address range into offset range
-  _bm.at_put_range(heapWordToOffset(mr.start()),
-                   heapWordToOffset(mr.end()), false);
+  _bm.at_put_range(addr_to_offset(intersection.start()),
+                   addr_to_offset(intersection.end()), false);
 }
 
 G1CMMarkStack::G1CMMarkStack() :
@@ -438,8 +405,6 @@
   }
 
   assert(CGC_lock != NULL, "Where's the CGC_lock?");
-  assert(_markBitMap1.covers(g1h->reserved_region()), "_markBitMap1 inconsistency");
-  assert(_markBitMap2.covers(g1h->reserved_region()), "_markBitMap2 inconsistency");
 
   SATBMarkQueueSet& satb_qs = JavaThread::satb_mark_queue_set();
   satb_qs.set_buffer_size(G1SATBBufferSize);
@@ -753,7 +718,7 @@
 
 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);
+  clear_bitmap(_prevMarkBitMap, workers, false);
 }
 
 class CheckBitmapClearHRClosure : public HeapRegionClosure {
@@ -769,7 +734,7 @@
     // value passed to it as limit to detect any found bits.
     // end never changes in G1.
     HeapWord* end = r->end();
-    return _bitmap->getNextMarkedWordAddress(r->bottom(), end) != end;
+    return _bitmap->get_next_marked_addr(r->bottom(), end) != end;
   }
 };
 
@@ -789,7 +754,6 @@
 
 void G1ConcurrentMark::checkpointRootsInitialPre() {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
-  G1Policy* g1p = g1h->g1_policy();
 
   _has_aborted = false;
 
@@ -1764,9 +1728,9 @@
 }
 
 void G1ConcurrentMark::swapMarkBitMaps() {
-  G1CMBitMapRO* temp = _prevMarkBitMap;
-  _prevMarkBitMap    = (G1CMBitMapRO*)_nextMarkBitMap;
-  _nextMarkBitMap    = (G1CMBitMap*)  temp;
+  G1CMBitMap* temp = _prevMarkBitMap;
+  _prevMarkBitMap  = _nextMarkBitMap;
+  _nextMarkBitMap  = temp;
 }
 
 // Closure for marking entries in SATB buffers.
@@ -1911,9 +1875,7 @@
 }
 
 void G1ConcurrentMark::clearRangePrevBitmap(MemRegion mr) {
-  // Note we are overriding the read-only view of the prev map here, via
-  // the cast.
-  ((G1CMBitMap*)_prevMarkBitMap)->clear_range(mr);
+  _prevMarkBitMap->clear_range(mr);
 }
 
 HeapRegion*
@@ -2160,37 +2122,22 @@
   _nextMarkBitMap->print_on_error(st, " Next Bits: ");
 }
 
-// Closure for iteration over bitmaps
-class G1CMBitMapClosure : public BitMapClosure {
-private:
-  // the bitmap that is being iterated over
-  G1CMBitMap*                 _nextMarkBitMap;
-  G1ConcurrentMark*           _cm;
-  G1CMTask*                   _task;
-
-public:
-  G1CMBitMapClosure(G1CMTask *task, G1ConcurrentMark* cm, G1CMBitMap* nextMarkBitMap) :
-    _task(task), _cm(cm), _nextMarkBitMap(nextMarkBitMap) { }
-
-  bool do_bit(size_t offset) {
-    HeapWord* addr = _nextMarkBitMap->offsetToHeapWord(offset);
-    assert(_nextMarkBitMap->isMarked(addr), "invariant");
-    assert( addr < _cm->finger(), "invariant");
-    assert(addr >= _task->finger(), "invariant");
-
-    // We move that task's local finger along.
-    _task->move_finger_to(addr);
-
-    _task->scan_task_entry(G1TaskQueueEntry::from_oop(oop(addr)));
-    // we only partially drain the local queue and global stack
-    _task->drain_local_queue(true);
-    _task->drain_global_stack(true);
-
-    // if the has_aborted flag has been raised, we need to bail out of
-    // the iteration
-    return !_task->has_aborted();
-  }
-};
+bool G1CMBitMapClosure::do_addr(HeapWord* const addr) {
+  assert(addr < _cm->finger(), "invariant");
+  assert(addr >= _task->finger(), "invariant");
+
+  // We move that task's local finger along.
+  _task->move_finger_to(addr);
+
+  _task->scan_task_entry(G1TaskQueueEntry::from_oop(oop(addr)));
+  // we only partially drain the local queue and global stack
+  _task->drain_local_queue(true);
+  _task->drain_global_stack(true);
+
+  // if the has_aborted flag has been raised, we need to bail out of
+  // the iteration
+  return !_task->has_aborted();
+}
 
 static ReferenceProcessor* get_cm_oop_closure_ref_processor(G1CollectedHeap* g1h) {
   ReferenceProcessor* result = g1h->ref_processor_cm();
@@ -2691,7 +2638,7 @@
   // Set up the bitmap and oop closures. Anything that uses them is
   // eventually called from this method, so it is OK to allocate these
   // statically.
-  G1CMBitMapClosure bitmap_closure(this, _cm, _nextMarkBitMap);
+  G1CMBitMapClosure bitmap_closure(this, _cm);
   G1CMOopClosure    cm_oop_closure(_g1h, _cm, this);
   set_cm_oop_closure(&cm_oop_closure);
 
@@ -2747,10 +2694,9 @@
         giveup_current_region();
         regular_clock_call();
       } else if (_curr_region->is_humongous() && mr.start() == _curr_region->bottom()) {
-        if (_nextMarkBitMap->isMarked(mr.start())) {
+        if (_nextMarkBitMap->is_marked(mr.start())) {
           // The object is marked - apply the closure
-          BitMap::idx_t offset = _nextMarkBitMap->heapWordToOffset(mr.start());
-          bitmap_closure.do_bit(offset);
+          bitmap_closure.do_addr(mr.start());
         }
         // Even if this task aborted while scanning the humongous object
         // we can (and should) give up the current region.
@@ -2772,11 +2718,9 @@
         // points to the address of the object we last scanned. If we
         // leave it there, when we restart this task, we will rescan
         // the object. It is easy to avoid this. We move the finger by
-        // enough to point to the next possible object header (the
-        // bitmap knows by how much we need to move it as it knows its
-        // granularity).
+        // enough to point to the next possible object header.
         assert(_finger < _region_limit, "invariant");
-        HeapWord* new_finger = _nextMarkBitMap->nextObject(_finger);
+        HeapWord* const new_finger = _finger + ((oop)_finger)->size();
         // Check if bitmap iteration was aborted while scanning the last object
         if (new_finger >= _region_limit) {
           giveup_current_region();
--- a/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1ConcurrentMark.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -110,57 +110,15 @@
   bool do_object_b(oop obj);
 };
 
-// A generic CM bit map.  This is essentially a wrapper around the BitMap
-// class, with one bit per (1<<_shifter) HeapWords.
+// Closure for iteration over bitmaps
+class G1CMBitMapClosure VALUE_OBJ_CLASS_SPEC {
+private:
+  G1ConcurrentMark* const _cm;
+  G1CMTask* const _task;
+public:
+  G1CMBitMapClosure(G1CMTask *task, G1ConcurrentMark* cm) : _task(task), _cm(cm) { }
 
-class G1CMBitMapRO VALUE_OBJ_CLASS_SPEC {
- protected:
-  HeapWord*  _bmStartWord; // base address of range covered by map
-  size_t     _bmWordSize;  // map size (in #HeapWords covered)
-  const int  _shifter;     // map to char or bit
-  BitMapView _bm;          // the bit map itself
-
- public:
-  // constructor
-  G1CMBitMapRO(int shifter);
-
-  // inquiries
-  HeapWord* startWord()   const { return _bmStartWord; }
-  // the following is one past the last word in space
-  HeapWord* endWord()     const { return _bmStartWord + _bmWordSize; }
-
-  // read marks
-
-  bool isMarked(HeapWord* addr) const {
-    assert(_bmStartWord <= addr && addr < (_bmStartWord + _bmWordSize),
-           "outside underlying space?");
-    return _bm.at(heapWordToOffset(addr));
-  }
-
-  // iteration
-  inline bool iterate(BitMapClosure* cl, MemRegion mr);
-
-  // Return the address corresponding to the next marked bit at or after
-  // "addr", and before "limit", if "limit" is non-NULL.  If there is no
-  // such bit, returns "limit" if that is non-NULL, or else "endWord()".
-  HeapWord* getNextMarkedWordAddress(const HeapWord* addr,
-                                     const HeapWord* limit = NULL) const;
-
-  // conversion utilities
-  HeapWord* offsetToHeapWord(size_t offset) const {
-    return _bmStartWord + (offset << _shifter);
-  }
-  size_t heapWordToOffset(const HeapWord* addr) const {
-    return pointer_delta(addr, _bmStartWord) >> _shifter;
-  }
-
-  // The argument addr should be the start address of a valid object
-  inline HeapWord* nextObject(HeapWord* addr);
-
-  void print_on_error(outputStream* st, const char* prefix) const;
-
-  // debugging
-  NOT_PRODUCT(bool covers(MemRegion rs) const;)
+  bool do_addr(HeapWord* const addr);
 };
 
 class G1CMBitMapMappingChangedListener : public G1MappingChangedListener {
@@ -174,11 +132,29 @@
   virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled);
 };
 
-class G1CMBitMap : public G1CMBitMapRO {
- private:
+// A generic mark bitmap for concurrent marking.  This is essentially a wrapper
+// around the BitMap class that is based on HeapWords, with one bit per (1 << _shifter) HeapWords.
+class G1CMBitMap VALUE_OBJ_CLASS_SPEC {
+private:
+  MemRegion _covered;    // The heap area covered by this bitmap.
+
+  const int _shifter;    // Shift amount from heap index to bit index in the bitmap.
+
+  BitMapView _bm;        // The actual bitmap.
+
   G1CMBitMapMappingChangedListener _listener;
 
- public:
+  inline void check_mark(HeapWord* addr) NOT_DEBUG_RETURN;
+
+  // Convert from bit offset to address.
+  HeapWord* offset_to_addr(size_t offset) const {
+    return _covered.start() + (offset << _shifter);
+  }
+  // Convert from address to bit offset.
+  size_t addr_to_offset(const HeapWord* addr) const {
+    return pointer_delta(addr, _covered.start()) >> _shifter;
+  }
+public:
   static size_t compute_size(size_t heap_size);
   // Returns the amount of bytes on the heap between two marks in the bitmap.
   static size_t mark_distance();
@@ -188,15 +164,37 @@
     return mark_distance();
   }
 
-  G1CMBitMap() : G1CMBitMapRO(LogMinObjAlignment), _listener() { _listener.set_bitmap(this); }
+  G1CMBitMap() : _covered(), _bm(), _shifter(LogMinObjAlignment), _listener() { _listener.set_bitmap(this); }
 
   // Initializes the underlying BitMap to cover the given area.
   void initialize(MemRegion heap, G1RegionToSpaceMapper* storage);
 
+  // read marks
+  bool is_marked(HeapWord* addr) const {
+    assert(_covered.contains(addr),
+           "Address " PTR_FORMAT " is outside underlying space from " PTR_FORMAT " to " PTR_FORMAT,
+           p2i(addr), p2i(_covered.start()), p2i(_covered.end()));
+    return _bm.at(addr_to_offset(addr));
+  }
+
+  // Apply the closure to the addresses that correspond to marked bits in the bitmap.
+  inline bool iterate(G1CMBitMapClosure* cl, MemRegion mr);
+
+  // Return the address corresponding to the next marked bit at or after
+  // "addr", and before "limit", if "limit" is non-NULL.  If there is no
+  // such bit, returns "limit" if that is non-NULL, or else "endWord()".
+  inline HeapWord* get_next_marked_addr(const HeapWord* addr,
+                                        const HeapWord* limit) const;
+
+  // The argument addr should be the start address of a valid object
+  inline HeapWord* addr_after_obj(HeapWord* addr);
+
+  void print_on_error(outputStream* st, const char* prefix) const;
+
   // Write marks.
   inline void mark(HeapWord* addr);
   inline void clear(HeapWord* addr);
-  inline bool parMark(HeapWord* addr);
+  inline bool par_mark(HeapWord* addr);
 
   void clear_range(MemRegion mr);
 };
@@ -396,7 +394,7 @@
   // Concurrent marking support structures
   G1CMBitMap              _markBitMap1;
   G1CMBitMap              _markBitMap2;
-  G1CMBitMapRO*           _prevMarkBitMap; // Completed mark bitmap
+  G1CMBitMap*             _prevMarkBitMap; // Completed mark bitmap
   G1CMBitMap*             _nextMarkBitMap; // Under-construction mark bitmap
 
   // Heap bounds
@@ -626,8 +624,8 @@
 
   ConcurrentMarkThread* cmThread() { return _cmThread; }
 
-  G1CMBitMapRO* prevMarkBitMap() const { return _prevMarkBitMap; }
-  G1CMBitMap*   nextMarkBitMap() const { return _nextMarkBitMap; }
+  const G1CMBitMap* const prevMarkBitMap() const { return _prevMarkBitMap; }
+  G1CMBitMap* nextMarkBitMap() const { return _nextMarkBitMap; }
 
   // Returns the number of GC threads to be used in a concurrent
   // phase based on the number of GC threads being used in a STW
--- a/src/share/vm/gc/g1/g1ConcurrentMark.inline.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1ConcurrentMark.inline.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -33,64 +33,62 @@
 #include "utilities/bitMap.inline.hpp"
 
 inline bool G1ConcurrentMark::par_mark(oop obj) {
-  return _nextMarkBitMap->parMark((HeapWord*)obj);
+  return _nextMarkBitMap->par_mark((HeapWord*)obj);
 }
 
-inline bool G1CMBitMapRO::iterate(BitMapClosure* cl, MemRegion mr) {
-  HeapWord* start_addr = MAX2(startWord(), mr.start());
-  HeapWord* end_addr = MIN2(endWord(), mr.end());
+inline bool G1CMBitMap::iterate(G1CMBitMapClosure* cl, MemRegion mr) {
+  assert(!mr.is_empty(), "Does not support empty memregion to iterate over");
+  assert(_covered.contains(mr),
+         "Given MemRegion from " PTR_FORMAT " to " PTR_FORMAT " not contained in heap area",
+         p2i(mr.start()), p2i(mr.end()));
 
-  if (end_addr > start_addr) {
-    // Right-open interval [start-offset, end-offset).
-    BitMap::idx_t start_offset = heapWordToOffset(start_addr);
-    BitMap::idx_t end_offset = heapWordToOffset(end_addr);
+  BitMap::idx_t const end_offset = addr_to_offset(mr.end());
+  BitMap::idx_t offset = _bm.get_next_one_offset(addr_to_offset(mr.start()), end_offset);
 
-    start_offset = _bm.get_next_one_offset(start_offset, end_offset);
-    while (start_offset < end_offset) {
-      if (!cl->do_bit(start_offset)) {
-        return false;
-      }
-      HeapWord* next_addr = MIN2(nextObject(offsetToHeapWord(start_offset)), end_addr);
-      BitMap::idx_t next_offset = heapWordToOffset(next_addr);
-      start_offset = _bm.get_next_one_offset(next_offset, end_offset);
+  while (offset < end_offset) {
+    HeapWord* const addr = offset_to_addr(offset);
+    if (!cl->do_addr(addr)) {
+      return false;
     }
+    size_t const obj_size = (size_t)((oop)addr)->size();
+    offset = _bm.get_next_one_offset(offset + (obj_size >> _shifter), end_offset);
   }
   return true;
 }
 
-// The argument addr should be the start address of a valid object
-HeapWord* G1CMBitMapRO::nextObject(HeapWord* addr) {
-  oop obj = (oop) addr;
-  HeapWord* res =  addr + obj->size();
-  assert(offsetToHeapWord(heapWordToOffset(res)) == res, "sanity");
-  return res;
+inline HeapWord* G1CMBitMap::get_next_marked_addr(const HeapWord* addr,
+                                                  const HeapWord* limit) const {
+  assert(limit != NULL, "limit must not be NULL");
+  // Round addr up to a possible object boundary to be safe.
+  size_t const addr_offset = addr_to_offset(align_up(addr, HeapWordSize << _shifter));
+  size_t const limit_offset = addr_to_offset(limit);
+  size_t const nextOffset = _bm.get_next_one_offset(addr_offset, limit_offset);
+  return offset_to_addr(nextOffset);
 }
 
-#define check_mark(addr)                                                       \
-  assert(_bmStartWord <= (addr) && (addr) < (_bmStartWord + _bmWordSize),      \
-         "outside underlying space?");                                         \
-  assert(G1CollectedHeap::heap()->is_in_exact(addr),                           \
-         "Trying to access not available bitmap " PTR_FORMAT                   \
-         " corresponding to " PTR_FORMAT " (%u)",                              \
-         p2i(this), p2i(addr), G1CollectedHeap::heap()->addr_to_region(addr));
+#ifdef ASSERT
+inline void G1CMBitMap::check_mark(HeapWord* addr) {
+  assert(G1CollectedHeap::heap()->is_in_exact(addr),
+         "Trying to access bitmap " PTR_FORMAT " for address " PTR_FORMAT " not in the heap.",
+         p2i(this), p2i(addr));
+}
+#endif
 
 inline void G1CMBitMap::mark(HeapWord* addr) {
   check_mark(addr);
-  _bm.set_bit(heapWordToOffset(addr));
+  _bm.set_bit(addr_to_offset(addr));
 }
 
 inline void G1CMBitMap::clear(HeapWord* addr) {
   check_mark(addr);
-  _bm.clear_bit(heapWordToOffset(addr));
+  _bm.clear_bit(addr_to_offset(addr));
 }
 
-inline bool G1CMBitMap::parMark(HeapWord* addr) {
+inline bool G1CMBitMap::par_mark(HeapWord* addr) {
   check_mark(addr);
-  return _bm.par_set_bit(heapWordToOffset(addr));
+  return _bm.par_set_bit(addr_to_offset(addr));
 }
 
-#undef check_mark
-
 #ifndef PRODUCT
 template<typename Fn>
 inline void G1CMMarkStack::iterate(Fn fn) const {
@@ -122,7 +120,7 @@
   assert(task_entry.is_array_slice() || !_g1h->is_on_master_free_list(
               _g1h->heap_region_containing(task_entry.obj())), "invariant");
   assert(task_entry.is_array_slice() || !_g1h->is_obj_ill(task_entry.obj()), "invariant");  // FIXME!!!
-  assert(task_entry.is_array_slice() || _nextMarkBitMap->isMarked((HeapWord*)task_entry.obj()), "invariant");
+  assert(task_entry.is_array_slice() || _nextMarkBitMap->is_marked((HeapWord*)task_entry.obj()), "invariant");
 
   if (!_task_queue->push(task_entry)) {
     // The local task queue looks full. We need to push some entries
@@ -170,7 +168,7 @@
 template<bool scan>
 inline void G1CMTask::process_grey_task_entry(G1TaskQueueEntry task_entry) {
   assert(scan || (task_entry.is_oop() && task_entry.obj()->is_typeArray()), "Skipping scan of grey non-typeArray");
-  assert(task_entry.is_array_slice() || _nextMarkBitMap->isMarked((HeapWord*)task_entry.obj()),
+  assert(task_entry.is_array_slice() || _nextMarkBitMap->is_marked((HeapWord*)task_entry.obj()),
          "Any stolen object should be a slice or marked");
 
   if (scan) {
@@ -240,7 +238,7 @@
   assert(obj->is_oop_or_null(true /* ignore mark word */), "Expected an oop or NULL at " PTR_FORMAT, p2i(obj));
   if (_g1h->is_in_g1_reserved(objAddr)) {
     assert(obj != NULL, "null check is implicit");
-    if (!_nextMarkBitMap->isMarked(objAddr)) {
+    if (!_nextMarkBitMap->is_marked(objAddr)) {
       // Only get the containing region if the object is not marked on the
       // bitmap (otherwise, it's a waste of time since we won't do
       // anything with it).
@@ -253,19 +251,13 @@
 }
 
 inline void G1ConcurrentMark::markPrev(oop p) {
-  assert(!_prevMarkBitMap->isMarked((HeapWord*) p), "sanity");
-  // Note we are overriding the read-only view of the prev map here, via
-  // the cast.
-  ((G1CMBitMap*)_prevMarkBitMap)->mark((HeapWord*) p);
+  assert(!_prevMarkBitMap->is_marked((HeapWord*) p), "sanity");
+ _prevMarkBitMap->mark((HeapWord*) p);
 }
 
 bool G1ConcurrentMark::isPrevMarked(oop p) const {
   assert(p != NULL && p->is_oop(), "expected an oop");
-  HeapWord* addr = (HeapWord*)p;
-  assert(addr >= _prevMarkBitMap->startWord() ||
-         addr < _prevMarkBitMap->endWord(), "in a region");
-
-  return _prevMarkBitMap->isMarked(addr);
+  return _prevMarkBitMap->is_marked((HeapWord*)p);
 }
 
 inline void G1ConcurrentMark::grayRoot(oop obj, HeapRegion* hr) {
@@ -282,7 +274,7 @@
   assert(!hr->is_continues_humongous(), "sanity");
 
   if (addr < hr->next_top_at_mark_start()) {
-    if (!_nextMarkBitMap->isMarked(addr)) {
+    if (!_nextMarkBitMap->is_marked(addr)) {
       par_mark(obj);
     }
   }
--- a/src/share/vm/gc/g1/g1HeapVerifier.cpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1HeapVerifier.cpp	Fri Aug 04 14:15:42 2017 +0200
@@ -598,11 +598,11 @@
   _g1h->collection_set()->iterate(&cl);
 }
 
-bool G1HeapVerifier::verify_no_bits_over_tams(const char* bitmap_name, G1CMBitMapRO* bitmap,
+bool G1HeapVerifier::verify_no_bits_over_tams(const char* bitmap_name, const G1CMBitMap* const bitmap,
                                                HeapWord* tams, HeapWord* end) {
   guarantee(tams <= end,
             "tams: " PTR_FORMAT " end: " PTR_FORMAT, p2i(tams), p2i(end));
-  HeapWord* result = bitmap->getNextMarkedWordAddress(tams, end);
+  HeapWord* result = bitmap->get_next_marked_addr(tams, end);
   if (result < end) {
     log_error(gc, verify)("## wrong marked address on %s bitmap: " PTR_FORMAT, bitmap_name, p2i(result));
     log_error(gc, verify)("## %s tams: " PTR_FORMAT " end: " PTR_FORMAT, bitmap_name, p2i(tams), p2i(end));
@@ -612,10 +612,9 @@
 }
 
 bool G1HeapVerifier::verify_bitmaps(const char* caller, HeapRegion* hr) {
-  G1CMBitMapRO* prev_bitmap = _g1h->concurrent_mark()->prevMarkBitMap();
-  G1CMBitMapRO* next_bitmap = (G1CMBitMapRO*) _g1h->concurrent_mark()->nextMarkBitMap();
+  const G1CMBitMap* const prev_bitmap = _g1h->concurrent_mark()->prevMarkBitMap();
+  const G1CMBitMap* const next_bitmap = _g1h->concurrent_mark()->nextMarkBitMap();
 
-  HeapWord* bottom = hr->bottom();
   HeapWord* ptams  = hr->prev_top_at_mark_start();
   HeapWord* ntams  = hr->next_top_at_mark_start();
   HeapWord* end    = hr->end();
--- a/src/share/vm/gc/g1/g1HeapVerifier.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/g1HeapVerifier.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -82,7 +82,7 @@
   // range [from,limit). If it does, print an error message and return
   // false. Otherwise, just return true. bitmap_name should be "prev"
   // or "next".
-  bool verify_no_bits_over_tams(const char* bitmap_name, G1CMBitMapRO* bitmap,
+  bool verify_no_bits_over_tams(const char* bitmap_name, const G1CMBitMap* const bitmap,
                                 HeapWord* from, HeapWord* limit);
 
   // Verify that the prev / next bitmap range [tams,end) for the given
--- a/src/share/vm/gc/g1/heapRegion.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/heapRegion.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -56,7 +56,7 @@
 // room for filler objects to pad out to the end of the region.
 
 class G1CollectedHeap;
-class G1CMBitMapRO;
+class G1CMBitMap;
 class HeapRegionRemSet;
 class HeapRegionRemSetIterator;
 class HeapRegion;
@@ -225,7 +225,7 @@
   // May
   // - only called with obj < top()
   // - not called on humongous objects or archive regions
-  inline bool is_obj_dead_with_size(const oop obj, G1CMBitMapRO* prev_bitmap, size_t* size) const;
+  inline bool is_obj_dead_with_size(const oop obj, const G1CMBitMap* const prev_bitmap, size_t* size) const;
 
  protected:
   // The index of this region in the heap region sequence.
@@ -301,7 +301,7 @@
 
   // Returns the block size of the given (dead, potentially having its class unloaded) object
   // starting at p extending to at most the prev TAMS using the given mark bitmap.
-  inline size_t block_size_using_bitmap(const HeapWord* p, const G1CMBitMapRO* prev_bitmap) const;
+  inline size_t block_size_using_bitmap(const HeapWord* p, const G1CMBitMap* const prev_bitmap) const;
  public:
   HeapRegion(uint hrm_index,
              G1BlockOffsetTable* bot,
@@ -349,7 +349,7 @@
   bool block_is_obj(const HeapWord* p) const;
 
   // Returns whether the given object is dead based on TAMS and bitmap.
-  bool is_obj_dead(const oop obj, const G1CMBitMapRO* prev_bitmap) const;
+  bool is_obj_dead(const oop obj, const G1CMBitMap* const prev_bitmap) const;
 
   // Returns the object size for all valid block starts
   // and the amount of unallocated words if called on top()
--- a/src/share/vm/gc/g1/heapRegion.inline.hpp	Thu Aug 03 22:40:26 2017 -0700
+++ b/src/share/vm/gc/g1/heapRegion.inline.hpp	Fri Aug 04 14:15:42 2017 +0200
@@ -113,7 +113,7 @@
   return _bot_part.block_start_const(p);
 }
 
-inline bool HeapRegion::is_obj_dead_with_size(const oop obj, G1CMBitMapRO* prev_bitmap, size_t* size) const {
+inline bool HeapRegion::is_obj_dead_with_size(const oop obj, const G1CMBitMap* const prev_bitmap, size_t* size) const {
   HeapWord* addr = (HeapWord*) obj;
 
   assert(addr < top(), "must be");
@@ -145,7 +145,7 @@
   return p < top();
 }
 
-inline size_t HeapRegion::block_size_using_bitmap(const HeapWord* addr, const G1CMBitMapRO* prev_bitmap) const {
+inline size_t HeapRegion::block_size_using_bitmap(const HeapWord* addr, const G1CMBitMap* const prev_bitmap) const {
   assert(ClassUnloadingWithConcurrentMark,
          "All blocks should be objects if class unloading isn't used, so this method should not be called. "
          "HR: [" PTR_FORMAT ", " PTR_FORMAT ", " PTR_FORMAT ") "
@@ -154,15 +154,15 @@
 
   // Old regions' dead objects may have dead classes
   // We need to find the next live object using the bitmap
-  HeapWord* next = prev_bitmap->getNextMarkedWordAddress(addr, prev_top_at_mark_start());
+  HeapWord* next = prev_bitmap->get_next_marked_addr(addr, prev_top_at_mark_start());
 
   assert(next > addr, "must get the next live object");
   return pointer_delta(next, addr);
 }
 
-inline bool HeapRegion::is_obj_dead(const oop obj, const G1CMBitMapRO* prev_bitmap) const {
+inline bool HeapRegion::is_obj_dead(const oop obj, const G1CMBitMap* const prev_bitmap) const {
   assert(is_in_reserved(obj), "Object " PTR_FORMAT " must be in region", p2i(obj));
-  return !obj_allocated_since_prev_marking(obj) && !prev_bitmap->isMarked((HeapWord*)obj);
+  return !obj_allocated_since_prev_marking(obj) && !prev_bitmap->is_marked((HeapWord*)obj);
 }
 
 inline size_t HeapRegion::block_size(const HeapWord *addr) const {
@@ -331,7 +331,7 @@
   }
 #endif
 
-  G1CMBitMapRO* bitmap = g1h->concurrent_mark()->prevMarkBitMap();
+  const G1CMBitMap* const bitmap = g1h->concurrent_mark()->prevMarkBitMap();
   do {
     oop obj = oop(cur);
     assert(obj->is_oop(true), "Not an oop at " PTR_FORMAT, p2i(cur));