8369068: GenShen: Generations still aren't reconciled assertion failure

Reviewed-by: ysr, kdnilsen

Author: William Kemper

src/hotspot/share/gc/shenandoah/heuristics/shenandoahGenerationalHeuristics.cpp

@@ -127,7 +127,7 @@ void ShenandoahGenerationalHeuristics::choose_collection_set(ShenandoahCollectio
       // Reclaim humongous regions here, and count them as the immediate garbage
 #ifdef ASSERT
       bool reg_live = region->has_live();
-      bool bm_live = heap->active_generation()->complete_marking_context()->is_marked(cast_to_oop(region->bottom()));
+      bool bm_live = _generation->complete_marking_context()->is_marked(cast_to_oop(region->bottom()));
       assert(reg_live == bm_live,
              "Humongous liveness and marks should agree. Region live: %s; Bitmap live: %s; Region Live Words: %zu",
              BOOL_TO_STR(reg_live), BOOL_TO_STR(bm_live), region->get_live_data_words());

src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.cpp

@@ -73,10 +73,11 @@ ShenandoahHeuristics::~ShenandoahHeuristics() {
 }
 
 void ShenandoahHeuristics::choose_collection_set(ShenandoahCollectionSet* collection_set) {
-  assert(collection_set->is_empty(), "Must be empty");
-
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
+  assert(collection_set->is_empty(), "Must be empty");
+  assert(!heap->mode()->is_generational(), "Wrong heuristic for heap mode");
+
   // Check all pinned regions have updated status before choosing the collection set.
   heap->assert_pinned_region_status();
 
@@ -120,7 +121,7 @@ void ShenandoahHeuristics::choose_collection_set(ShenandoahCollectionSet* collec
       // Reclaim humongous regions here, and count them as the immediate garbage
 #ifdef ASSERT
       bool reg_live = region->has_live();
-      bool bm_live = heap->gc_generation()->complete_marking_context()->is_marked(cast_to_oop(region->bottom()));
+      bool bm_live = heap->global_generation()->complete_marking_context()->is_marked(cast_to_oop(region->bottom()));
       assert(reg_live == bm_live,
              "Humongous liveness and marks should agree. Region live: %s; Bitmap live: %s; Region Live Words: %zu",
              BOOL_TO_STR(reg_live), BOOL_TO_STR(bm_live), region->get_live_data_words());

src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp

@@ -425,6 +425,16 @@ void ShenandoahAsserts::assert_marked_strong(void *interior_loc, oop obj, const
   }
 }
 
+void ShenandoahAsserts::assert_mark_complete(HeapWord* obj, const char* file, int line) {
+  const ShenandoahHeap* heap = ShenandoahHeap::heap();
+  const ShenandoahHeapRegion* region = heap->heap_region_containing(obj);
+  const ShenandoahGeneration* generation = heap->generation_for(region->affiliation());
+  if (!generation->is_mark_complete()) {
+    ShenandoahMessageBuffer msg("Marking should be complete for object " PTR_FORMAT " in the %s generation", p2i(obj), generation->name());
+    report_vm_error(file, line, msg.buffer());
+  }
+}
+
 void ShenandoahAsserts::assert_in_cset(void* interior_loc, oop obj, const char* file, int line) {
   assert_correct(interior_loc, obj, file, line);
 
@@ -542,23 +552,6 @@ void ShenandoahAsserts::assert_control_or_vm_thread_at_safepoint(bool at_safepoi
   report_vm_error(file, line, msg.buffer());
 }
 
-void ShenandoahAsserts::assert_generations_reconciled(const char* file, int line) {
-  if (!ShenandoahSafepoint::is_at_shenandoah_safepoint()) {
-    // Only shenandoah safepoint operations participate in the active/gc generation scheme
-    return;
-  }
-
-  ShenandoahHeap* heap = ShenandoahHeap::heap();
-  ShenandoahGeneration* ggen = heap->gc_generation();
-  ShenandoahGeneration* agen = heap->active_generation();
-  if (agen == ggen) {
-    return;
-  }
-
-  ShenandoahMessageBuffer msg("Active(%s) & GC(%s) Generations aren't reconciled", agen->name(), ggen->name());
-  report_vm_error(file, line, msg.buffer());
-}
-
 bool ShenandoahAsserts::extract_klass_safely(oop obj, narrowKlass& nk, const Klass*& k) {
   nk = 0;
   k = nullptr;

src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp

@@ -65,6 +65,9 @@ class ShenandoahAsserts {
   static void assert_marked(void* interior_loc, oop obj, const char* file, int line);
   static void assert_marked_weak(void* interior_loc, oop obj, const char* file, int line);
   static void assert_marked_strong(void* interior_loc, oop obj, const char* file, int line);
+
+  // Assert that marking is complete for the generation where this obj resides
+  static void assert_mark_complete(HeapWord* obj, const char* file, int line);
   static void assert_in_cset(void* interior_loc, oop obj, const char* file, int line);
   static void assert_not_in_cset(void* interior_loc, oop obj, const char* file, int line);
   static void assert_not_in_cset_loc(void* interior_loc, const char* file, int line);
@@ -76,7 +79,6 @@ class ShenandoahAsserts {
   static void assert_heaplocked_or_safepoint(const char* file, int line);
   static void assert_control_or_vm_thread_at_safepoint(bool at_safepoint, const char* file, int line);
   static void assert_generational(const char* file, int line);
-  static void assert_generations_reconciled(const char* file, int line);
 
   // Given a possibly invalid oop, extract narrowKlass (if UCCP) and Klass*
   // from it safely.
@@ -133,6 +135,9 @@ class ShenandoahAsserts {
 #define shenandoah_assert_marked_strong(interior_loc, obj) \
                     ShenandoahAsserts::assert_marked_strong(interior_loc, obj, __FILE__, __LINE__)
 
+#define shenandoah_assert_mark_complete(obj) \
+                    ShenandoahAsserts::assert_mark_complete(obj, __FILE__, __LINE__)
+
 #define shenandoah_assert_in_cset_if(interior_loc, obj, condition) \
   if (condition)    ShenandoahAsserts::assert_in_cset(interior_loc, obj, __FILE__, __LINE__)
 #define shenandoah_assert_in_cset_except(interior_loc, obj, exception) \
@@ -184,10 +189,6 @@ class ShenandoahAsserts {
 #define shenandoah_assert_generational() \
                     ShenandoahAsserts::assert_generational(__FILE__, __LINE__)
 
-// Some limited sanity checking of the _gc_generation and _active_generation fields of ShenandoahHeap
-#define shenandoah_assert_generations_reconciled()                                                             \
-                    ShenandoahAsserts::assert_generations_reconciled(__FILE__, __LINE__)
-
 #else
 #define shenandoah_assert_in_heap_bounds(interior_loc, obj)
 #define shenandoah_assert_in_heap_bounds_or_null(interior_loc, obj)
@@ -217,6 +218,8 @@ class ShenandoahAsserts {
 #define shenandoah_assert_marked_strong_except(interior_loc, obj, exception)
 #define shenandoah_assert_marked_strong(interior_loc, obj)
 
+#define shenandoah_assert_mark_complete(obj)
+
 #define shenandoah_assert_in_cset_if(interior_loc, obj, condition)
 #define shenandoah_assert_in_cset_except(interior_loc, obj, exception)
 #define shenandoah_assert_in_cset(interior_loc, obj)
@@ -241,7 +244,6 @@ class ShenandoahAsserts {
 #define shenandoah_assert_control_or_vm_thread()
 #define shenandoah_assert_control_or_vm_thread_at_safepoint()
 #define shenandoah_assert_generational()
-#define shenandoah_assert_generations_reconciled()
 
 #endif
 

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp

@@ -91,8 +91,8 @@ class ShenandoahBreakpointMarkScope : public StackObj {
 };
 
 ShenandoahConcurrentGC::ShenandoahConcurrentGC(ShenandoahGeneration* generation, bool do_old_gc_bootstrap) :
+  ShenandoahGC(generation),
   _mark(generation),
-  _generation(generation),
   _degen_point(ShenandoahDegenPoint::_degenerated_unset),
   _abbreviated(false),
   _do_old_gc_bootstrap(do_old_gc_bootstrap) {
@@ -576,7 +576,7 @@ void ShenandoahConcurrentGC::entry_promote_in_place() const {
   ShenandoahGCWorkerPhase worker_phase(ShenandoahPhaseTimings::promote_in_place);
   EventMark em("%s", "Promote in place");
 
-  ShenandoahGenerationalHeap::heap()->promote_regions_in_place(true);
+  ShenandoahGenerationalHeap::heap()->promote_regions_in_place(_generation, true);
 }
 
 void ShenandoahConcurrentGC::entry_update_thread_roots() {
@@ -706,7 +706,7 @@ void ShenandoahConcurrentGC::op_init_mark() {
 
   if (ShenandoahVerify) {
     ShenandoahTimingsTracker v(ShenandoahPhaseTimings::init_mark_verify);
-    heap->verifier()->verify_before_concmark();
+    heap->verifier()->verify_before_concmark(_generation);
   }
 
   if (VerifyBeforeGC) {
@@ -763,7 +763,7 @@ void ShenandoahConcurrentGC::op_final_mark() {
   assert(!heap->has_forwarded_objects(), "No forwarded objects on this path");
 
   if (ShenandoahVerify) {
-    heap->verifier()->verify_roots_no_forwarded();
+    heap->verifier()->verify_roots_no_forwarded(_generation);
   }
 
   if (!heap->cancelled_gc()) {
@@ -791,7 +791,7 @@ void ShenandoahConcurrentGC::op_final_mark() {
 
       if (ShenandoahVerify) {
         ShenandoahTimingsTracker v(ShenandoahPhaseTimings::final_mark_verify);
-        heap->verifier()->verify_before_evacuation();
+        heap->verifier()->verify_before_evacuation(_generation);
       }
 
       heap->set_evacuation_in_progress(true);
@@ -806,9 +806,9 @@ void ShenandoahConcurrentGC::op_final_mark() {
       if (ShenandoahVerify) {
         ShenandoahTimingsTracker v(ShenandoahPhaseTimings::final_mark_verify);
         if (has_in_place_promotions(heap)) {
-          heap->verifier()->verify_after_concmark_with_promotions();
+          heap->verifier()->verify_after_concmark_with_promotions(_generation);
         } else {
-          heap->verifier()->verify_after_concmark();
+          heap->verifier()->verify_after_concmark(_generation);
         }
       }
     }
@@ -877,18 +877,20 @@ void ShenandoahConcurrentGC::op_weak_refs() {
 class ShenandoahEvacUpdateCleanupOopStorageRootsClosure : public BasicOopIterateClosure {
 private:
   ShenandoahHeap* const _heap;
+  ShenandoahGeneration* const _generation;
   ShenandoahMarkingContext* const _mark_context;
   bool  _evac_in_progress;
   Thread* const _thread;
 
 public:
-  ShenandoahEvacUpdateCleanupOopStorageRootsClosure();
+  explicit ShenandoahEvacUpdateCleanupOopStorageRootsClosure(ShenandoahGeneration* generation);
   void do_oop(oop* p);
   void do_oop(narrowOop* p);
 };
 
-ShenandoahEvacUpdateCleanupOopStorageRootsClosure::ShenandoahEvacUpdateCleanupOopStorageRootsClosure() :
+ShenandoahEvacUpdateCleanupOopStorageRootsClosure::ShenandoahEvacUpdateCleanupOopStorageRootsClosure(ShenandoahGeneration* generation) :
   _heap(ShenandoahHeap::heap()),
+  _generation(generation),
   _mark_context(ShenandoahHeap::heap()->marking_context()),
   _evac_in_progress(ShenandoahHeap::heap()->is_evacuation_in_progress()),
   _thread(Thread::current()) {
@@ -898,8 +900,7 @@ void ShenandoahEvacUpdateCleanupOopStorageRootsClosure::do_oop(oop* p) {
   const oop obj = RawAccess<>::oop_load(p);
   if (!CompressedOops::is_null(obj)) {
     if (!_mark_context->is_marked(obj)) {
-      shenandoah_assert_generations_reconciled();
-      if (_heap->is_in_active_generation(obj)) {
+      if (_generation->contains(obj)) {
         // Note: The obj is dead here. Do not touch it, just clear.
         ShenandoahHeap::atomic_clear_oop(p, obj);
       }
@@ -942,29 +943,31 @@ class ShenandoahConcurrentWeakRootsEvacUpdateTask : public WorkerTask {
   ShenandoahClassLoaderDataRoots<true /* concurrent */>
                                              _cld_roots;
   ShenandoahConcurrentNMethodIterator        _nmethod_itr;
+  ShenandoahGeneration*                      _generation;
   ShenandoahPhaseTimings::Phase              _phase;
 
 public:
-  ShenandoahConcurrentWeakRootsEvacUpdateTask(ShenandoahPhaseTimings::Phase phase) :
+  ShenandoahConcurrentWeakRootsEvacUpdateTask(ShenandoahGeneration* generation, ShenandoahPhaseTimings::Phase phase) :
     WorkerTask("Shenandoah Evacuate/Update Concurrent Weak Roots"),
     _vm_roots(phase),
     _cld_roots(phase, ShenandoahHeap::heap()->workers()->active_workers(), false /*heap iteration*/),
     _nmethod_itr(ShenandoahCodeRoots::table()),
+    _generation(generation),
     _phase(phase) {}
 
   ~ShenandoahConcurrentWeakRootsEvacUpdateTask() {
     // Notify runtime data structures of potentially dead oops
     _vm_roots.report_num_dead();
   }
 
-  void work(uint worker_id) {
+  void work(uint worker_id) override {
     ShenandoahConcurrentWorkerSession worker_session(worker_id);
     ShenandoahSuspendibleThreadSetJoiner sts_join;
     {
       ShenandoahEvacOOMScope oom;
       // jni_roots and weak_roots are OopStorage backed roots, concurrent iteration
       // may race against OopStorage::release() calls.
-      ShenandoahEvacUpdateCleanupOopStorageRootsClosure cl;
+      ShenandoahEvacUpdateCleanupOopStorageRootsClosure cl(_generation);
       _vm_roots.oops_do(&cl, worker_id);
     }
 
@@ -999,7 +1002,7 @@ void ShenandoahConcurrentGC::op_weak_roots() {
     // Concurrent weak root processing
     ShenandoahTimingsTracker t(ShenandoahPhaseTimings::conc_weak_roots_work);
     ShenandoahGCWorkerPhase worker_phase(ShenandoahPhaseTimings::conc_weak_roots_work);
-    ShenandoahConcurrentWeakRootsEvacUpdateTask task(ShenandoahPhaseTimings::conc_weak_roots_work);
+    ShenandoahConcurrentWeakRootsEvacUpdateTask task(_generation, ShenandoahPhaseTimings::conc_weak_roots_work);
     heap->workers()->run_task(&task);
   }
 
@@ -1105,19 +1108,19 @@ void ShenandoahConcurrentGC::op_cleanup_early() {
 }
 
 void ShenandoahConcurrentGC::op_evacuate() {
-  ShenandoahHeap::heap()->evacuate_collection_set(true /*concurrent*/);
+  ShenandoahHeap::heap()->evacuate_collection_set(_generation, true /*concurrent*/);
 }
 
 void ShenandoahConcurrentGC::op_init_update_refs() {
-  ShenandoahHeap* const heap = ShenandoahHeap::heap();
   if (ShenandoahVerify) {
+    ShenandoahHeap* const heap = ShenandoahHeap::heap();
     ShenandoahTimingsTracker v(ShenandoahPhaseTimings::init_update_refs_verify);
-    heap->verifier()->verify_before_update_refs();
+    heap->verifier()->verify_before_update_refs(_generation);
   }
 }
 
 void ShenandoahConcurrentGC::op_update_refs() {
-  ShenandoahHeap::heap()->update_heap_references(true /*concurrent*/);
+  ShenandoahHeap::heap()->update_heap_references(_generation, true /*concurrent*/);
 }
 
 class ShenandoahUpdateThreadHandshakeClosure : public HandshakeClosure {
@@ -1163,7 +1166,7 @@ void ShenandoahConcurrentGC::op_final_update_refs() {
 
   // Has to be done before cset is clear
   if (ShenandoahVerify) {
-    heap->verifier()->verify_roots_in_to_space();
+    heap->verifier()->verify_roots_in_to_space(_generation);
   }
 
   // If we are running in generational mode and this is an aging cycle, this will also age active
@@ -1198,7 +1201,7 @@ void ShenandoahConcurrentGC::op_final_update_refs() {
 
   if (ShenandoahVerify) {
     ShenandoahTimingsTracker v(ShenandoahPhaseTimings::final_update_refs_verify);
-    heap->verifier()->verify_after_update_refs();
+    heap->verifier()->verify_after_update_refs(_generation);
   }
 
   if (VerifyAfterGC) {

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp

@@ -47,7 +47,6 @@ class ShenandoahConcurrentGC : public ShenandoahGC {
 
 protected:
   ShenandoahConcurrentMark    _mark;
-  ShenandoahGeneration* const _generation;
 
 private:
   ShenandoahDegenPoint        _degen_point;

src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp

@@ -56,18 +56,11 @@ class ShenandoahConcurrentMarkingTask : public WorkerTask {
   }
 
   void work(uint worker_id) {
-    ShenandoahHeap* heap = ShenandoahHeap::heap();
     ShenandoahConcurrentWorkerSession worker_session(worker_id);
     ShenandoahWorkerTimingsTracker timer(ShenandoahPhaseTimings::conc_mark, ShenandoahPhaseTimings::ParallelMark, worker_id, true);
     ShenandoahSuspendibleThreadSetJoiner stsj;
-    // Do not use active_generation() : we must use the gc_generation() set by
-    // ShenandoahGCScope on the ControllerThread's stack; no safepoint may
-    // intervene to update active_generation, so we can't
-    // shenandoah_assert_generations_reconciled() here.
-    ShenandoahReferenceProcessor* rp = heap->gc_generation()->ref_processor();
-    assert(rp != nullptr, "need reference processor");
     StringDedup::Requests requests;
-    _cm->mark_loop(worker_id, _terminator, rp, GENERATION, true /*cancellable*/,
+    _cm->mark_loop(worker_id, _terminator, GENERATION, true /*cancellable*/,
                    ShenandoahStringDedup::is_enabled() ? ENQUEUE_DEDUP : NO_DEDUP,
                    &requests);
   }
@@ -106,9 +99,6 @@ class ShenandoahFinalMarkingTask : public WorkerTask {
 
     ShenandoahParallelWorkerSession worker_session(worker_id);
     StringDedup::Requests requests;
-    ShenandoahReferenceProcessor* rp = heap->gc_generation()->ref_processor();
-    shenandoah_assert_generations_reconciled();
-
     // First drain remaining SATB buffers.
     {
       ShenandoahObjToScanQueue* q = _cm->get_queue(worker_id);
@@ -122,7 +112,7 @@ class ShenandoahFinalMarkingTask : public WorkerTask {
       ShenandoahSATBAndRemarkThreadsClosure tc(satb_mq_set);
       Threads::possibly_parallel_threads_do(true /* is_par */, &tc);
     }
-    _cm->mark_loop(worker_id, _terminator, rp, GENERATION, false /*not cancellable*/,
+    _cm->mark_loop(worker_id, _terminator, GENERATION, false /*not cancellable*/,
                    _dedup_string ? ENQUEUE_DEDUP : NO_DEDUP,
                    &requests);
     assert(_cm->task_queues()->is_empty(), "Should be empty");