Remove _top_at_mark_start HeapRegion member

Author: Thomas Schatzl

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

@@ -748,7 +748,7 @@ class G1ClearBitMapTask : public WorkerTask {
       }
       assert(cur >= end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index());
 
-      r->reset_top_at_mark_start();
+      _cm->reset_top_at_mark_start(r);
 
       return false;
     }
@@ -860,9 +860,15 @@ void G1PreConcurrentStartTask::ResetMarkingStateTask::do_work(uint worker_id) {
 }
 
 class NoteStartOfMarkHRClosure : public HeapRegionClosure {
+  G1ConcurrentMark* _cm;
+
 public:
+  NoteStartOfMarkHRClosure() : HeapRegionClosure(), _cm(G1CollectedHeap::heap()->concurrent_mark()) { }
+
   bool do_heap_region(HeapRegion* r) override {
-    r->note_start_of_marking();
+    if (r->is_old_or_humongous() && !r->is_collection_set_candidate()) {
+      _cm->update_top_at_mark_start(r);
+    }
     return false;
   }
 };
@@ -1261,7 +1267,7 @@ class G1UpdateRemSetTrackingBeforeRebuildTask : public WorkerTask {
     }
 
     void add_marked_bytes_and_note_end(HeapRegion* hr, size_t marked_bytes) {
-      hr->note_end_of_marking(marked_bytes);
+      hr->note_end_of_marking(_cm->top_at_mark_start(hr), marked_bytes);
       _cl->do_heap_region(hr);
     }
 
@@ -1936,6 +1942,7 @@ void G1ConcurrentMark::flush_all_task_caches() {
 void G1ConcurrentMark::clear_bitmap_for_region(HeapRegion* hr) {
   assert_at_safepoint();
   _mark_bitmap.clear_range(MemRegion(hr->bottom(), hr->end()));
+  reset_top_at_mark_start(hr);
 }
 
 HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {

src/hotspot/share/gc/g1/g1ConcurrentMark.hpp

@@ -564,7 +564,7 @@ class G1ConcurrentMark : public CHeapObj<mtGC> {
 
   inline void update_top_at_mark_start(HeapRegion* r);
   inline void reset_top_at_mark_start(HeapRegion* r);
-  inline HeapWord* top_at_mark_start(HeapRegion* r) const;
+  inline HeapWord* top_at_mark_start(const HeapRegion* r) const;
   inline HeapWord* top_at_mark_start(uint region) const;
   inline bool obj_allocated_since_mark_start(oop obj) const;
 

src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp

@@ -187,23 +187,15 @@ inline size_t G1CMTask::scan_objArray(objArrayOop obj, MemRegion mr) {
 inline void G1ConcurrentMark::update_top_at_mark_start(HeapRegion* r) {
   uint const region = r->hrm_index();
   assert(region < _g1h->max_reserved_regions(), "Tried to access TAMS for region %u out of bounds", region);
-  assert(_top_at_mark_starts[region] == r->bottom(),
-         "TAMS for region %u has already been set to " PTR_FORMAT " should be bottom " PTR_FORMAT,
-         region, p2i(_top_at_mark_starts[region]), p2i(r->bottom()));
   _top_at_mark_starts[region] = r->top();
 }
 
 inline void G1ConcurrentMark::reset_top_at_mark_start(HeapRegion* r) {
   _top_at_mark_starts[r->hrm_index()] = r->bottom();
 }
 
-inline HeapWord* G1ConcurrentMark::top_at_mark_start(HeapRegion* r) const {
-  HeapWord* value = top_at_mark_start(r->hrm_index());
-  assert(r->top_at_mark_start() == top_at_mark_start(r->hrm_index()),
-         "Inconsistency check region %u %s " PTR_FORMAT " " PTR_FORMAT,
-         r->hrm_index(), r->get_short_type_str(), p2i(r->top_at_mark_start()), p2i(top_at_mark_start(r->hrm_index())   )      );
-
-  return value;
+inline HeapWord* G1ConcurrentMark::top_at_mark_start(const HeapRegion* r) const {
+  return top_at_mark_start(r->hrm_index());
 }
 
 inline HeapWord* G1ConcurrentMark::top_at_mark_start(uint region) const {
@@ -214,9 +206,6 @@ inline HeapWord* G1ConcurrentMark::top_at_mark_start(uint region) const {
 inline bool G1ConcurrentMark::obj_allocated_since_mark_start(oop obj) const {
   uint const region = _g1h->addr_to_region(obj);
   assert(region < _g1h->max_reserved_regions(), "obj " PTR_FORMAT " outside heap %u", p2i(obj), region);
-  assert(_g1h->region_at(region)->top_at_mark_start() == top_at_mark_start(region),
-         "Inconsistency region %u %s " PTR_FORMAT " " PTR_FORMAT, region, _g1h->region_at(region)->get_short_type_str(), p2i(_g1h->region_at(region)->top_at_mark_start()), p2i(top_at_mark_start(region))
-          );
   return cast_from_oop<HeapWord*>(obj) >= top_at_mark_start(region);
 }
 

src/hotspot/share/gc/g1/g1HeapRegion.cpp

@@ -124,7 +124,10 @@ void HeapRegion::hr_clear(bool clear_space) {
 
   rem_set()->clear();
 
-  init_top_at_mark_start();
+  G1CollectedHeap::heap()->concurrent_mark()->reset_top_at_mark_start(this); // FIXME
+  _parsable_bottom = bottom();
+  _garbage_bytes = 0;
+
   if (clear_space) clear(SpaceDecorator::Mangle);
 }
 
@@ -226,7 +229,6 @@ HeapRegion::HeapRegion(uint hrm_index,
 #ifdef ASSERT
   _containing_set(nullptr),
 #endif
-  _top_at_mark_start(nullptr),
   _parsable_bottom(nullptr),
   _garbage_bytes(0),
   _young_index_in_cset(-1),
@@ -414,8 +416,9 @@ void HeapRegion::print_on(outputStream* st) const {
   } else {
     st->print("|  ");
   }
+  G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark();
   st->print("|TAMS " PTR_FORMAT "| PB " PTR_FORMAT "| %s ",
-            p2i(top_at_mark_start()), p2i(parsable_bottom_acquire()), rem_set()->get_state_str());
+            p2i(cm->top_at_mark_start(this)), p2i(parsable_bottom_acquire()), rem_set()->get_state_str());
   if (UseNUMA) {
     G1NUMA* numa = G1NUMA::numa();
     if (node_index() < numa->num_active_nodes()) {

src/hotspot/share/gc/g1/g1HeapRegion.hpp

@@ -223,12 +223,6 @@ class HeapRegion : public CHeapObj<mtGC> {
   HeapRegionSetBase* _containing_set;
 #endif // ASSERT
 
-  // The start of the unmarked area. The unmarked area extends from this
-  // word until the top and/or end of the region, and is the part
-  // of the region for which no marking was done, i.e. objects may
-  // have been allocated in this part since the last mark phase.
-  HeapWord* volatile _top_at_mark_start;
-
   // The area above this limit is fully parsable. This limit
   // is equal to bottom except
   //
@@ -246,8 +240,6 @@ class HeapRegion : public CHeapObj<mtGC> {
   // Amount of dead data in the region.
   size_t _garbage_bytes;
 
-  inline void init_top_at_mark_start();
-
   // Data for young region survivor prediction.
   uint  _young_index_in_cset;
   G1SurvRateGroup* _surv_rate_group;
@@ -352,10 +344,6 @@ class HeapRegion : public CHeapObj<mtGC> {
 
   inline bool is_collection_set_candidate() const;
 
-  // Get the start of the unmarked area in this region.
-  HeapWord* top_at_mark_start() const;
-  void set_top_at_mark_start(HeapWord* value);
-
   // Retrieve parsable bottom; since it may be modified concurrently, outside a
   // safepoint the _acquire method must be used.
   HeapWord* parsable_bottom() const;
@@ -366,20 +354,13 @@ class HeapRegion : public CHeapObj<mtGC> {
   // that the collector is about to start or has finished (concurrently)
   // marking the heap.
 
-  // Notify the region that concurrent marking is starting. Initialize
-  // all fields related to the next marking info.
-  inline void note_start_of_marking();
-
-  // Notify the region that concurrent marking has finished. Passes the number of
-  // bytes between bottom and TAMS.
-  inline void note_end_of_marking(size_t marked_bytes);
+  // Notify the region that concurrent marking has finished. Passes TAMS and the number of
+  // bytes marked between bottom and TAMS.
+  inline void note_end_of_marking(HeapWord* top_at_mark_start, size_t marked_bytes);
 
   // Notify the region that scrubbing has completed.
   inline void note_end_of_scrubbing();
 
-  // Notify the region that the (corresponding) bitmap has been cleared.
-  inline void reset_top_at_mark_start();
-
   // During the concurrent scrubbing phase, can there be any areas with unloaded
   // classes or dead objects in this region?
   // This set only includes old regions - humongous regions only

src/hotspot/share/gc/g1/g1HeapRegion.inline.hpp

@@ -176,27 +176,26 @@ inline void HeapRegion::prepare_for_full_gc() {
 
 inline void HeapRegion::reset_compacted_after_full_gc(HeapWord* new_top) {
   set_top(new_top);
-  // After a compaction the mark bitmap in a movable region is invalid.
-  // But all objects are live, we get this by setting TAMS to bottom.
-  init_top_at_mark_start();
 
   reset_after_full_gc_common();
 }
 
 inline void HeapRegion::reset_skip_compacting_after_full_gc() {
   assert(!is_free(), "must be");
 
-  _garbage_bytes = 0;
-
-  reset_top_at_mark_start();
-
   reset_after_full_gc_common();
 }
 
 inline void HeapRegion::reset_after_full_gc_common() {
+  // After a full gc the mark bitmap in a movable region is invalid.
+  // But all objects are live, we get this by setting TAMS to bottom.
+  G1CollectedHeap::heap()->concurrent_mark()->reset_top_at_mark_start(this); // FIXME
+
   // Everything above bottom() is parsable and live.
   reset_parsable_bottom();
 
+  _garbage_bytes = 0;
+
   // Clear unused heap memory in debug builds.
   if (ZapUnusedHeapArea) {
     mangle_unused_area();
@@ -266,21 +265,6 @@ inline void HeapRegion::update_bot_for_obj(HeapWord* obj_start, size_t obj_size)
   _bot_part.update_for_block(obj_start, obj_end);
 }
 
-inline HeapWord* HeapRegion::top_at_mark_start() const {
-  HeapWord* value = Atomic::load(&_top_at_mark_start);
-  assert(value == G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(hrm_index()),
-         "must be equal region %u %s " PTR_FORMAT " " PTR_FORMAT,
-          hrm_index(), get_short_type_str(),
-         p2i(value), p2i(G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(hrm_index())));
-  return value;
-}
-
-inline void HeapRegion::set_top_at_mark_start(HeapWord* value) {
-  Atomic::store(&_top_at_mark_start, value);
-  assert(_top_at_mark_start == G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(this),
-         "duh " PTR_FORMAT " " PTR_FORMAT, p2i(_top_at_mark_start), p2i(G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(this)));
-}
-
 inline HeapWord* HeapRegion::parsable_bottom() const {
   assert(!is_init_completed() || SafepointSynchronize::is_at_safepoint(), "only during initialization or safepoint");
   return _parsable_bottom;
@@ -294,48 +278,22 @@ inline void HeapRegion::reset_parsable_bottom() {
   Atomic::release_store(&_parsable_bottom, bottom());
 }
 
-inline void HeapRegion::note_start_of_marking() {
-  assert(top_at_mark_start() == bottom(), "Region's TAMS must always be at bottom");
-  if (is_old_or_humongous() && !is_collection_set_candidate()) {
-    G1CollectedHeap::heap()->concurrent_mark()->update_top_at_mark_start(this);
-    set_top_at_mark_start(top());
-  }
-}
-
-inline void HeapRegion::note_end_of_marking(size_t marked_bytes) {
+inline void HeapRegion::note_end_of_marking(HeapWord* top_at_mark_start, size_t marked_bytes) {
   assert_at_safepoint();
 
-  if (top_at_mark_start() != bottom()) {
-    _garbage_bytes = byte_size(bottom(), top_at_mark_start()) - marked_bytes;
+  if (top_at_mark_start != bottom()) {
+    _garbage_bytes = byte_size(bottom(), top_at_mark_start) - marked_bytes;
   }
 
   if (needs_scrubbing()) {
-    _parsable_bottom = top_at_mark_start();
+    _parsable_bottom = top_at_mark_start;
   }
 }
 
 inline void HeapRegion::note_end_of_scrubbing() {
   reset_parsable_bottom();
 }
 
-inline void HeapRegion::init_top_at_mark_start() {
-  reset_top_at_mark_start();
-  _parsable_bottom = bottom();
-  _garbage_bytes = 0;
-}
-
-inline void HeapRegion::reset_top_at_mark_start() {
-  // We do not need a release store here because
-  //
-  // - if this method is called during concurrent bitmap clearing, we do not read
-  // the bitmap any more for live/dead information (we do not read the bitmap at
-  // all at that point).
-  // - otherwise we reclaim regions only during GC and we do not read tams and the
-  // bitmap concurrently.
-  G1CollectedHeap::heap()->concurrent_mark()->reset_top_at_mark_start(this);
-  set_top_at_mark_start(bottom());
-}
-
 inline bool HeapRegion::needs_scrubbing() const {
   return is_old();
 }

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

@@ -1133,7 +1133,6 @@ class G1MergeHeapRootsTask : public WorkerTask {
       // so the bitmap for the regions in the collection set must be cleared if not already.
       if (should_clear_region(hr)) {
         _g1h->clear_bitmap_for_region(hr);
-        hr->reset_top_at_mark_start();
       } else {
         assert_bitmap_clear(hr, _g1h->concurrent_mark()->mark_bitmap());
       }

src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.cpp

@@ -56,14 +56,15 @@ void G1RemSetTrackingPolicy::update_at_free(HeapRegion* r) {
 }
 
 static void print_before_rebuild(HeapRegion* r, bool selected_for_rebuild, size_t total_live_bytes, size_t live_bytes) {
+  G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark();
   log_trace(gc, remset, tracking)("Before rebuild region %u "
                                   "(tams: " PTR_FORMAT ") "
                                   "total_live_bytes %zu "
                                   "selected %s "
                                   "(live_bytes %zu "
                                   "type %s)",
                                   r->hrm_index(),
-                                  p2i(r->top_at_mark_start()),
+                                  p2i(cm->top_at_mark_start(r)),
                                   total_live_bytes,
                                   BOOL_TO_STR(selected_for_rebuild),
                                   live_bytes,
@@ -152,7 +153,7 @@ void G1RemSetTrackingPolicy::update_after_rebuild(HeapRegion* r) {
                                     "remset occ %zu "
                                     "size %zu)",
                                     r->hrm_index(),
-                                    p2i(r->top_at_mark_start()),
+                                    p2i(cm->top_at_mark_start(r)),
                                     cm->live_bytes(r->hrm_index()),
                                     r->rem_set()->occupied(),
                                     r->rem_set()->mem_size());

src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp

@@ -86,7 +86,7 @@ static inline bool requires_marking(const void* entry, G1CollectedHeap* g1h) {
          "Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry));
 
   G1ConcurrentMark* cm = g1h->concurrent_mark();
-  if (cm->obj_allocated_since_mark_start(entry)) {
+  if (cm->obj_allocated_since_mark_start(cast_to_oop(entry))) {
     return false;
   }
 

src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp

@@ -560,9 +560,8 @@ class G1PostEvacuateCollectionSetCleanupTask2::ProcessEvacuationFailedRegionsTas
       } else {
         // This evacuation failed region is going to be marked through. Update mark data.
         cm->update_top_at_mark_start(r);
-        r->set_top_at_mark_start(r->top());
         cm->set_live_bytes(r->hrm_index(), r->live_bytes());
-        assert(cm->mark_bitmap()->get_next_marked_addr(r->bottom(), r->top_at_mark_start()) != r->top_at_mark_start(),
+        assert(cm->mark_bitmap()->get_next_marked_addr(r->bottom(), cm->top_at_mark_start(r)) != cm->top_at_mark_start(r),
                "Marks must be on bitmap for region %u", r->hrm_index());
       }
       return false;