8262973: Verify ParCompactionManager instance in PCAdjustPointerClosure

Reviewed-by: kbarrett, tschatzl

Author: albertnetymk

src/hotspot/share/gc/parallel/psCompactionManager.cpp

@@ -179,3 +179,19 @@ void ParCompactionManager::push_shadow_region(size_t shadow_region) {
 void ParCompactionManager::remove_all_shadow_regions() {
   _shadow_region_array->clear();
 }
+
+#ifdef ASSERT
+void ParCompactionManager::verify_all_marking_stack_empty() {
+  uint parallel_gc_threads = ParallelGCThreads;
+  for (uint i = 0; i <= parallel_gc_threads; i++) {
+    assert(_manager_array[i]->marking_stacks_empty(), "Marking stack should be empty");
+  }
+}
+
+void ParCompactionManager::verify_all_region_stack_empty() {
+  uint parallel_gc_threads = ParallelGCThreads;
+  for (uint i = 0; i <= parallel_gc_threads; i++) {
+    assert(_manager_array[i]->region_stack()->is_empty(), "Region stack should be empty");
+  }
+}
+#endif

src/hotspot/share/gc/parallel/psCompactionManager.hpp

@@ -46,10 +46,6 @@ class ParCompactionManager : public CHeapObj<mtGC> {
   friend class PCRefProcTask;
   friend class MarkFromRootsTask;
   friend class UpdateDensePrefixAndCompactionTask;
-
- public:
-
-
  private:
   typedef GenericTaskQueue<oop, mtGC>             OopTaskQueue;
   typedef GenericTaskQueueSet<OopTaskQueue, mtGC> OopTaskQueueSet;
@@ -69,7 +65,6 @@ class ParCompactionManager : public CHeapObj<mtGC> {
   static RegionTaskQueueSet*    _region_task_queues;
   static PSOldGen*              _old_gen;
 
-private:
   OverflowTaskQueue<oop, mtGC>        _marking_stack;
   ObjArrayTaskQueue             _objarray_stack;
   size_t                        _next_shadow_region;
@@ -143,7 +138,7 @@ class ParCompactionManager : public CHeapObj<mtGC> {
 
   RegionTaskQueue* region_stack()                { return &_region_stack; }
 
-  inline static ParCompactionManager* manager_array(uint index);
+  static ParCompactionManager* get_vmthread_cm() { return _manager_array[ParallelGCThreads]; }
 
   ParCompactionManager();
 
@@ -196,13 +191,13 @@ class ParCompactionManager : public CHeapObj<mtGC> {
     FollowStackClosure(ParCompactionManager* cm) : _compaction_manager(cm) { }
     virtual void do_void();
   };
-};
 
-inline ParCompactionManager* ParCompactionManager::manager_array(uint index) {
-  assert(_manager_array != NULL, "access of NULL manager_array");
-  assert(index <= ParallelGCThreads, "out of range manager_array access");
-  return _manager_array[index];
-}
+  // Called after marking.
+  static void verify_all_marking_stack_empty() NOT_DEBUG_RETURN;
+
+  // Region staks hold regions in from-space; called after compaction.
+  static void verify_all_region_stack_empty() NOT_DEBUG_RETURN;
+};
 
 bool ParCompactionManager::marking_stacks_empty() const {
   return _marking_stack.is_empty() && _objarray_stack.is_empty();

src/hotspot/share/gc/parallel/psParallelCompact.cpp

@@ -1784,8 +1784,7 @@ bool PSParallelCompact::invoke_no_policy(bool maximum_heap_compaction) {
   const PreGenGCValues pre_gc_values = heap->get_pre_gc_values();
 
   // Get the compaction manager reserved for the VM thread.
-  ParCompactionManager* const vmthread_cm =
-    ParCompactionManager::manager_array(ParallelScavengeHeap::heap()->workers().total_workers());
+  ParCompactionManager* const vmthread_cm = ParCompactionManager::get_vmthread_cm();
 
   {
     const uint active_workers =
@@ -1837,6 +1836,8 @@ bool PSParallelCompact::invoke_no_policy(bool maximum_heap_compaction) {
     compaction_start.update();
     compact();
 
+    ParCompactionManager::verify_all_region_stack_empty();
+
     // Reset the mark bitmap, summary data, and do other bookkeeping.  Must be
     // done before resizing.
     post_compact();
@@ -1933,15 +1934,6 @@ bool PSParallelCompact::invoke_no_policy(bool maximum_heap_compaction) {
     heap->post_full_gc_dump(&_gc_timer);
   }
 
-#ifdef ASSERT
-  for (size_t i = 0; i < ParallelGCThreads + 1; ++i) {
-    ParCompactionManager* const cm =
-      ParCompactionManager::manager_array(int(i));
-    assert(cm->marking_stack()->is_empty(),       "should be empty");
-    assert(cm->region_stack()->is_empty(), "Region stack " SIZE_FORMAT " is not empty", i);
-  }
-#endif // ASSERT
-
   if (VerifyAfterGC && heap->total_collections() >= VerifyGCStartAt) {
     Universe::verify("After GC");
   }
@@ -2181,7 +2173,7 @@ void PSParallelCompact::marking_phase(ParCompactionManager* cm,
   }
 
   // This is the point where the entire marking should have completed.
-  assert(cm->marking_stacks_empty(), "Marking should have completed");
+  ParCompactionManager::verify_all_marking_stack_empty();
 
   {
     GCTraceTime(Debug, gc, phases) tm("Weak Processing", &_gc_timer);
@@ -2207,6 +2199,19 @@ void PSParallelCompact::marking_phase(ParCompactionManager* cm,
   _gc_tracer.report_object_count_after_gc(is_alive_closure());
 }
 
+#ifdef ASSERT
+void PCAdjustPointerClosure::verify_cm(ParCompactionManager* cm) {
+  assert(cm != NULL, "associate ParCompactionManage should not be NULL");
+  auto vmthread_cm = ParCompactionManager::get_vmthread_cm();
+  if (Thread::current()->is_VM_thread()) {
+    assert(cm == vmthread_cm, "VM threads should use ParCompactionManager from get_vmthread_cm()");
+  } else {
+    assert(Thread::current()->is_GC_task_thread(), "Must be a GC thread");
+    assert(cm != vmthread_cm, "GC threads should use ParCompactionManager from gc_thread_compaction_manager()");
+  }
+}
+#endif
+
 class PSAdjustTask final : public AbstractGangTask {
   SubTasksDone                               _sub_tasks;
   WeakProcessor::Task                        _weak_proc_task;
@@ -2614,9 +2619,11 @@ void PSParallelCompact::compact() {
   }
 
   {
-    // Update the deferred objects, if any.  Any compaction manager can be used.
     GCTraceTime(Trace, gc, phases) tm("Deferred Updates", &_gc_timer);
-    ParCompactionManager* cm = ParCompactionManager::manager_array(0);
+    // Update the deferred objects, if any. In principle, any compaction
+    // manager can be used. However, since the current thread is VM thread, we
+    // use the rightful one to keep the verification logic happy.
+    ParCompactionManager* cm = ParCompactionManager::get_vmthread_cm();
     for (unsigned int id = old_space_id; id < last_space_id; ++id) {
       update_deferred_objects(cm, SpaceId(id));
     }

src/hotspot/share/gc/parallel/psParallelCompact.inline.hpp

@@ -126,7 +126,7 @@ inline void PSParallelCompact::adjust_pointer(T* p, ParCompactionManager* cm) {
 class PCAdjustPointerClosure: public BasicOopIterateClosure {
 public:
   PCAdjustPointerClosure(ParCompactionManager* cm) {
-    assert(cm != NULL, "associate ParCompactionManage should not be NULL");
+    verify_cm(cm);
     _cm = cm;
   }
   template <typename T> void do_oop_nv(T* p) { PSParallelCompact::adjust_pointer(p, _cm); }
@@ -136,6 +136,8 @@ class PCAdjustPointerClosure: public BasicOopIterateClosure {
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS; }
 private:
   ParCompactionManager* _cm;
+
+  static void verify_cm(ParCompactionManager* cm) NOT_DEBUG_RETURN;
 };
 
 #endif // SHARE_GC_PARALLEL_PSPARALLELCOMPACT_INLINE_HPP

test/hotspot/gtest/gc/parallel/test_psParallelCompact.cpp

@@ -49,8 +49,6 @@ TEST_VM(PSParallelCompact, print_generic_summary_data) {
   // end region.  The end region should not be printed because it
   // corresponds to the space after the end of the heap.
   ParallelScavengeHeap* heap = ParallelScavengeHeap::heap();
-  ParCompactionManager* const vmthread_cm =
-    ParCompactionManager::manager_array(ParallelGCThreads);
   HeapWord* begin_heap =
     (HeapWord*) heap->old_gen()->virtual_space()->low_boundary();
   HeapWord* end_heap =