8326781

initial version

initial draft

some improvmeents

Make things work

forgotten fix

Author: Thomas Schatzl

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

@@ -1506,6 +1506,7 @@ void G1CollectedHeap::ref_processing_init() {
   //     * Reference discovery will not need a barrier.
 
   // Concurrent Mark ref processor
+  _is_alive_closure_cm.set_concurrent_mark(concurrent_mark());
   _ref_processor_cm =
     new ReferenceProcessor(&_is_subject_to_discovery_cm,
                            ParallelGCThreads,                              // degree of mt processing

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

@@ -255,7 +255,7 @@ class G1PrintCollectionSetDetailClosure : public HeapRegionClosure {
     assert(r->in_collection_set(), "Region %u should be in collection set", r->hrm_index());
     _st->print_cr("  " HR_FORMAT ", TAMS: " PTR_FORMAT " PB: " PTR_FORMAT ", age: %4d",
                   HR_FORMAT_PARAMS(r),
-                  p2i(r->top_at_mark_start()),
+                  p2i(G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(r)),
                   p2i(r->parsable_bottom()),
                   r->has_surv_rate_group() ? checked_cast<int>(r->age_in_surv_rate_group()) : -1);
     return false;

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

@@ -77,6 +77,14 @@
 #include "utilities/growableArray.hpp"
 #include "utilities/powerOfTwo.hpp"
 
+G1CMIsAliveClosure::G1CMIsAliveClosure(G1CollectedHeap* g1h) :
+  _g1h(g1h), _cm(g1h->concurrent_mark()) { }
+
+void G1CMIsAliveClosure::set_concurrent_mark(G1ConcurrentMark* cm) {
+  assert(_cm == nullptr, "already set");
+  _cm = cm;
+}
+
 bool G1CMBitMapClosure::do_addr(HeapWord* const addr) {
   assert(addr < _cm->finger(), "invariant");
   assert(addr >= _task->finger(), "invariant");
@@ -501,6 +509,7 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
   _max_concurrent_workers(0),
 
   _region_mark_stats(NEW_C_HEAP_ARRAY(G1RegionMarkStats, _g1h->max_reserved_regions(), mtGC)),
+  _top_at_mark_starts(NEW_C_HEAP_ARRAY(HeapWord*, _g1h->max_reserved_regions(), mtGC)),
   _top_at_rebuild_starts(NEW_C_HEAP_ARRAY(HeapWord*, _g1h->max_reserved_regions(), mtGC)),
   _needs_remembered_set_rebuild(false)
 {
@@ -558,6 +567,7 @@ void G1ConcurrentMark::reset() {
 
   uint max_reserved_regions = _g1h->max_reserved_regions();
   for (uint i = 0; i < max_reserved_regions; i++) {
+    _top_at_mark_starts[i] = _g1h->bottom_addr_for_region(i);
     _top_at_rebuild_starts[i] = nullptr;
     _region_mark_stats[i].clear();
   }
@@ -570,6 +580,7 @@ void G1ConcurrentMark::clear_statistics(HeapRegion* r) {
   for (uint j = 0; j < _max_num_tasks; ++j) {
     _tasks[j]->clear_mark_stats_cache(region_idx);
   }
+  _top_at_mark_starts[region_idx] = _g1h->bottom_addr_for_region(region_idx);
   _top_at_rebuild_starts[region_idx] = nullptr;
   _region_mark_stats[region_idx].clear();
 }
@@ -647,6 +658,7 @@ void G1ConcurrentMark::reset_at_marking_complete() {
 }
 
 G1ConcurrentMark::~G1ConcurrentMark() {
+  FREE_C_HEAP_ARRAY(HeapWord*, _top_at_mark_starts);
   FREE_C_HEAP_ARRAY(HeapWord*, _top_at_rebuild_starts);
   FREE_C_HEAP_ARRAY(G1RegionMarkStats, _region_mark_stats);
   // The G1ConcurrentMark instance is never freed.
@@ -692,7 +704,7 @@ class G1ClearBitMapTask : public WorkerTask {
           assert(_bitmap->get_next_marked_addr(r->bottom(), r->end()) == r->end(), "Should not have marked bits");
           return r->bottom();
         }
-        assert(_bitmap->get_next_marked_addr(r->top_at_mark_start(), r->end()) == r->end(), "Should not have marked bits above tams");
+        assert(_bitmap->get_next_marked_addr(_cm->top_at_mark_start(r), r->end()) == r->end(), "Should not have marked bits above tams");
       }
       return r->end();
     }
@@ -844,7 +856,7 @@ class G1PreConcurrentStartTask::NoteStartOfMarkTask : public G1AbstractSubTask {
 
 void G1PreConcurrentStartTask::ResetMarkingStateTask::do_work(uint worker_id) {
   // Reset marking state.
-  _cm->reset();
+  //_cm->reset();
 }
 
 class NoteStartOfMarkHRClosure : public HeapRegionClosure {
@@ -877,6 +889,7 @@ void G1ConcurrentMark::pre_concurrent_start(GCCause::Cause cause) {
 
   ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_strong);
 
+  reset();
   G1PreConcurrentStartTask cl(cause, this);
   G1CollectedHeap::heap()->run_batch_task(&cl);
 
@@ -1014,9 +1027,9 @@ void G1ConcurrentMark::scan_root_region(const MemRegion* region, uint worker_id)
 #ifdef ASSERT
   HeapWord* last = region->last();
   HeapRegion* hr = _g1h->heap_region_containing(last);
-  assert(hr->is_old() || hr->top_at_mark_start() == hr->bottom(),
+  assert(hr->is_old() || top_at_mark_start(hr) == hr->bottom(),
          "Root regions must be old or survivor/eden but region %u is %s", hr->hrm_index(), hr->get_type_str());
-  assert(hr->top_at_mark_start() == region->start(),
+  assert(top_at_mark_start(hr) == region->start(),
          "MemRegion start should be equal to TAMS");
 #endif
 
@@ -1078,11 +1091,11 @@ bool G1ConcurrentMark::wait_until_root_region_scan_finished() {
 }
 
 void G1ConcurrentMark::add_root_region(HeapRegion* r) {
-  root_regions()->add(r->top_at_mark_start(), r->top());
+  root_regions()->add(top_at_mark_start(r), r->top());
 }
 
 bool G1ConcurrentMark::is_root_region(HeapRegion* r) {
-  return root_regions()->contains(MemRegion(r->top_at_mark_start(), r->top()));
+  return root_regions()->contains(MemRegion(top_at_mark_start(r), r->top()));
 }
 
 void G1ConcurrentMark::root_region_scan_abort_and_wait() {
@@ -1944,9 +1957,9 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
     if (res == finger && curr_region != nullptr) {
       // we succeeded
       HeapWord* bottom = curr_region->bottom();
-      HeapWord* limit = curr_region->top_at_mark_start();
+      HeapWord* limit = top_at_mark_start(curr_region);
 
-      log_trace(gc, marking)("Claim region %u bottom " PTR_FORMAT " tams " PTR_FORMAT, curr_region->hrm_index(), p2i(curr_region->bottom()), p2i(curr_region->top_at_mark_start()));
+      log_trace(gc, marking)("Claim region %u bottom " PTR_FORMAT " tams " PTR_FORMAT, curr_region->hrm_index(), p2i(curr_region->bottom()), p2i(top_at_mark_start(curr_region)));
       // notice that _finger == end cannot be guaranteed here since,
       // someone else might have moved the finger even further
       assert(_finger >= end, "the finger should have moved forward");
@@ -2169,7 +2182,7 @@ void G1CMTask::setup_for_region(HeapRegion* hr) {
 void G1CMTask::update_region_limit() {
   HeapRegion* hr = _curr_region;
   HeapWord* bottom = hr->bottom();
-  HeapWord* limit = hr->top_at_mark_start();
+  HeapWord* limit = _cm->top_at_mark_start(hr);
 
   if (limit == bottom) {
     // The region was collected underneath our feet.

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

@@ -97,8 +97,11 @@ typedef GenericTaskQueueSet<G1CMTaskQueue, mtGC> G1CMTaskQueueSet;
 // reference processor as the _is_alive_non_header field
 class G1CMIsAliveClosure : public BoolObjectClosure {
   G1CollectedHeap* _g1h;
+  G1ConcurrentMark* _cm;
 public:
-  G1CMIsAliveClosure(G1CollectedHeap* g1h) : _g1h(g1h) { }
+  G1CMIsAliveClosure(G1CollectedHeap* g1h);
+  void set_concurrent_mark(G1ConcurrentMark* cm);
+
   bool do_object_b(oop obj);
 };
 
@@ -539,6 +542,7 @@ class G1ConcurrentMark : public CHeapObj<mtGC> {
 
   // Region statistics gathered during marking.
   G1RegionMarkStats* _region_mark_stats;
+  HeapWord* volatile* _top_at_mark_starts;
   // Top pointer for each region at the start of the rebuild remembered set process
   // for regions which remembered sets need to be rebuilt. A null for a given region
   // means that this region does not be scanned during the rebuilding remembered
@@ -558,10 +562,16 @@ class G1ConcurrentMark : public CHeapObj<mtGC> {
   // Set live bytes for concurrent marking.
   void set_live_bytes(uint region, size_t live_bytes) { _region_mark_stats[region]._live_words = live_bytes / HeapWordSize; }
 
+  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(uint region) const;
+  inline bool obj_allocated_since_mark_start(oop obj) const;
+
   // Sets the internal top_at_region_start for the given region to current top of the region.
   inline void update_top_at_rebuild_start(HeapRegion* r);
   // TARS for the given region during remembered set rebuilding.
-  inline HeapWord* top_at_rebuild_start(uint region) const;
+  inline HeapWord* top_at_rebuild_start(HeapRegion* r) const;
 
   // Clear statistics gathered during the concurrent cycle for the given region after
   // it has been reclaimed.

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

@@ -48,9 +48,8 @@ inline bool G1CMIsAliveClosure::do_object_b(oop obj) {
     return true;
   }
 
-  HeapRegion* hr = _g1h->heap_region_containing(obj);
   // All objects allocated since the start of marking are considered live.
-  if (hr->obj_allocated_since_marking_start(obj)) {
+  if (_cm->obj_allocated_since_mark_start(obj)) {
     return true;
   }
 
@@ -66,15 +65,16 @@ inline bool G1CMSubjectToDiscoveryClosure::do_object_b(oop obj) {
 }
 
 inline bool G1ConcurrentMark::mark_in_bitmap(uint const worker_id, oop const obj) {
-  HeapRegion* const hr = _g1h->heap_region_containing(obj);
-
-  if (hr->obj_allocated_since_marking_start(obj)) {
+  if (obj_allocated_since_mark_start(obj)) {
     return false;
   }
 
   // Some callers may have stale objects to mark above TAMS after humongous reclaim.
   // Can't assert that this is a valid object at this point, since it might be in the process of being copied by another thread.
-  assert(!hr->is_continues_humongous(), "Should not try to mark object " PTR_FORMAT " in Humongous continues region %u above TAMS " PTR_FORMAT, p2i(obj), hr->hrm_index(), p2i(hr->top_at_mark_start()));
+  DEBUG_ONLY(HeapRegion* const hr = _g1h->heap_region_containing(obj);)
+  assert(!hr->is_continues_humongous(),
+         "Should not try to mark object " PTR_FORMAT " in Humongous continues region %u above TAMS " PTR_FORMAT,
+         p2i(obj), hr->hrm_index(), p2i(top_at_mark_start(hr)));
 
   bool success = _mark_bitmap.par_mark(obj);
   if (success) {
@@ -184,9 +184,44 @@ inline size_t G1CMTask::scan_objArray(objArrayOop obj, MemRegion mr) {
   return mr.word_size();
 }
 
-inline HeapWord* G1ConcurrentMark::top_at_rebuild_start(uint region) const {
+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(uint region) const {
   assert(region < _g1h->max_reserved_regions(), "Tried to access TARS for region %u out of bounds", region);
-  return _top_at_rebuild_starts[region];
+  return _top_at_mark_starts[region];
+}
+
+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);
+}
+
+inline HeapWord* G1ConcurrentMark::top_at_rebuild_start(HeapRegion* r) const {
+  return _top_at_rebuild_starts[r->hrm_index()];
 }
 
 inline void G1ConcurrentMark::update_top_at_rebuild_start(HeapRegion* r) {

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

@@ -105,7 +105,7 @@ class G1RebuildRSAndScrubTask : public WorkerTask {
     //  - been allocated after rebuild start, or
     //  - been reclaimed by a collection.
     bool should_rebuild_or_scrub(HeapRegion* hr) const {
-      return _cm->top_at_rebuild_start(hr->hrm_index()) != nullptr;
+      return _cm->top_at_rebuild_start(hr) != nullptr;
     }
 
     // Helper used by both humongous objects and when chunking an object larger than the
@@ -229,7 +229,7 @@ class G1RebuildRSAndScrubTask : public WorkerTask {
       assert(should_rebuild_or_scrub(hr), "must be");
 
       log_trace(gc, marking)("Scrub and rebuild region: " HR_FORMAT " pb: " PTR_FORMAT " TARS: " PTR_FORMAT " TAMS: " PTR_FORMAT,
-                             HR_FORMAT_PARAMS(hr), p2i(pb), p2i(_cm->top_at_rebuild_start(hr->hrm_index())), p2i(hr->top_at_mark_start()));
+                             HR_FORMAT_PARAMS(hr), p2i(pb), p2i(_cm->top_at_rebuild_start(hr)), p2i(_cm->top_at_mark_start(hr)));
 
       if (scan_and_scrub_to_pb(hr, hr->bottom(), pb)) {
         log_trace(gc, marking)("Scan and scrub aborted for region: %u", hr->hrm_index());
@@ -246,7 +246,7 @@ class G1RebuildRSAndScrubTask : public WorkerTask {
       hr->note_end_of_scrubbing();
 
       // Rebuild from TAMS (= parsable_bottom) to TARS.
-      if (scan_from_pb_to_tars(hr, pb, _cm->top_at_rebuild_start(hr->hrm_index()))) {
+      if (scan_from_pb_to_tars(hr, pb, _cm->top_at_rebuild_start(hr))) {
         log_trace(gc, marking)("Rebuild aborted for region: %u (%s)", hr->hrm_index(), hr->get_short_type_str());
         return true;
       }
@@ -272,7 +272,7 @@ class G1RebuildRSAndScrubTask : public WorkerTask {
              "Humongous object not live");
 
       log_trace(gc, marking)("Rebuild for humongous region: " HR_FORMAT " pb: " PTR_FORMAT " TARS: " PTR_FORMAT,
-                              HR_FORMAT_PARAMS(hr), p2i(pb), p2i(_cm->top_at_rebuild_start(hr->hrm_index())));
+                              HR_FORMAT_PARAMS(hr), p2i(pb), p2i(_cm->top_at_rebuild_start(hr)));
 
       // Scan the humongous object in chunks from bottom to top to rebuild remembered sets.
       HeapWord* humongous_end = hr->humongous_start_region()->bottom() + humongous->size();

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

@@ -540,10 +540,6 @@ class HeapRegion : public CHeapObj<mtGC> {
   inline        bool is_in_parsable_area(const void* const addr) const;
   inline static bool is_in_parsable_area(const void* const addr, const void* const pb);
 
-  bool obj_allocated_since_marking_start(oop obj) const {
-    return cast_from_oop<HeapWord*>(obj) >= top_at_mark_start();
-  }
-
   // Update the region state after a failed evacuation.
   void handle_evacuation_failure(bool retain);
 

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

@@ -267,11 +267,18 @@ inline void HeapRegion::update_bot_for_obj(HeapWord* obj_start, size_t obj_size)
 }
 
 inline HeapWord* HeapRegion::top_at_mark_start() const {
-  return Atomic::load(&_top_at_mark_start);
+  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 {
@@ -290,6 +297,7 @@ inline void HeapRegion::reset_parsable_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());
   }
 }
@@ -324,6 +332,7 @@ inline void HeapRegion::reset_top_at_mark_start() {
   // 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());
 }
 

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

@@ -462,10 +462,12 @@ class G1VerifyRegionMarkingStateClosure : public HeapRegionClosure {
     G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark();
 
     bool part_of_marking = r->is_old_or_humongous() && !r->is_collection_set_candidate();
+    HeapWord* top_at_mark_start = cm->top_at_mark_start(r);
 
     if (part_of_marking) {
-      guarantee(r->bottom() != r->top_at_mark_start(), "region %u (%s) does not have TAMS set",
-                                                       r->hrm_index(), r->get_short_type_str());
+      guarantee(r->bottom() != top_at_mark_start,
+                "region %u (%s) does not have TAMS set",
+                r->hrm_index(), r->get_short_type_str());
       size_t marked_bytes = cm->live_bytes(r->hrm_index());
 
       MarkedBytesClosure cl;
@@ -475,9 +477,9 @@ class G1VerifyRegionMarkingStateClosure : public HeapRegionClosure {
                 "region %u (%s) live bytes actual %zu and cache %zu differ",
                 r->hrm_index(), r->get_short_type_str(), cl.marked_bytes(), marked_bytes);
     } else {
-      guarantee(r->bottom() == r->top_at_mark_start(),
+      guarantee(r->bottom() == top_at_mark_start,
                 "region %u (%s) has TAMS set " PTR_FORMAT " " PTR_FORMAT,
-                r->hrm_index(), r->get_short_type_str(), p2i(r->bottom()), p2i(r->top_at_mark_start()));
+                r->hrm_index(), r->get_short_type_str(), p2i(r->bottom()), p2i(top_at_mark_start));
       guarantee(cm->live_bytes(r->hrm_index()) == 0,
                 "region %u (%s) has %zu live bytes recorded",
                 r->hrm_index(), r->get_short_type_str(), cm->live_bytes(r->hrm_index()));
@@ -542,9 +544,10 @@ void G1HeapVerifier::verify_bitmap_clear(bool from_tams) {
     G1VerifyBitmapClear(bool from_tams) : _from_tams(from_tams) { }
 
     virtual bool do_heap_region(HeapRegion* r) {
-      G1CMBitMap* bitmap = G1CollectedHeap::heap()->concurrent_mark()->mark_bitmap();
+      G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark();
+      G1CMBitMap* bitmap = cm->mark_bitmap();
 
-      HeapWord* start = _from_tams ? r->top_at_mark_start() : r->bottom();
+      HeapWord* start = _from_tams ? cm->top_at_mark_start(r) : r->bottom();
 
       HeapWord* mark = bitmap->get_next_marked_addr(start, r->end());
       guarantee(mark == r->end(), "Found mark at " PTR_FORMAT " in region %u from start " PTR_FORMAT, p2i(mark), r->hrm_index(), p2i(start));

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

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1CollectionSetChooser.hpp"
 #include "gc/g1/g1HeapRegion.inline.hpp"
 #include "gc/g1/g1HeapRegionRemSet.inline.hpp"
@@ -101,7 +102,8 @@ bool G1RemSetTrackingPolicy::update_before_rebuild(HeapRegion* r, size_t live_by
 
   assert(!r->rem_set()->is_updating(), "Remembered set of region %u is updating before rebuild", r->hrm_index());
 
-  size_t live_bytes_above_tams = pointer_delta(r->top(), r->top_at_mark_start()) * HeapWordSize;
+  HeapWord* top_at_mark_start = G1CollectedHeap::heap()->concurrent_mark()->top_at_mark_start(r);
+  size_t live_bytes_above_tams = pointer_delta(r->top(), top_at_mark_start) * HeapWordSize;
   size_t total_live_bytes = live_bytes_below_tams + live_bytes_above_tams;
 
   bool selected_for_rebuild = false;