8321815: Shenandoah: gc state should be synchronized to java threads only once per safepoint

Reviewed-by: kdnilsen, ysr

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

@@ -504,6 +504,7 @@ ShenandoahHeap::ShenandoahHeap(ShenandoahCollectorPolicy* policy) :
   _num_regions(0),
   _regions(nullptr),
   _update_refs_iterator(this),
+  _gc_state_changed(false),
   _gc_no_progress_count(0),
   _control_thread(nullptr),
   _shenandoah_policy(policy),
@@ -1741,16 +1742,21 @@ void ShenandoahHeap::prepare_update_heap_references(bool concurrent) {
   _update_refs_iterator.reset();
 }
 
-void ShenandoahHeap::set_gc_state_all_threads(char state) {
-  for (JavaThreadIteratorWithHandle jtiwh; JavaThread *t = jtiwh.next(); ) {
-    ShenandoahThreadLocalData::set_gc_state(t, state);
+void ShenandoahHeap::set_gc_state_all_threads() {
+  assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint");
+  if (_gc_state_changed) {
+    _gc_state_changed = false;
+    char state = gc_state();
+    for (JavaThreadIteratorWithHandle jtiwh; JavaThread *t = jtiwh.next(); ) {
+      ShenandoahThreadLocalData::set_gc_state(t, state);
+    }
   }
 }
 
 void ShenandoahHeap::set_gc_state_mask(uint mask, bool value) {
-  assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Should really be Shenandoah safepoint");
+  assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint");
   _gc_state.set_cond(mask, value);
-  set_gc_state_all_threads(_gc_state.raw_value());
+  _gc_state_changed = true;
 }
 
 void ShenandoahHeap::set_concurrent_mark_in_progress(bool in_progress) {

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

@@ -124,6 +124,7 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {
   friend class ShenandoahGCStateResetter;
   friend class ShenandoahParallelObjectIterator;
   friend class ShenandoahSafepoint;
+
   // Supported GC
   friend class ShenandoahConcurrentGC;
   friend class ShenandoahDegenGC;
@@ -283,6 +284,7 @@ public:
   };
 
 private:
+  bool _gc_state_changed;
   ShenandoahSharedBitmap _gc_state;
   ShenandoahSharedFlag   _degenerated_gc_in_progress;
   ShenandoahSharedFlag   _full_gc_in_progress;
@@ -291,11 +293,12 @@ private:
 
   size_t _gc_no_progress_count;
 
-  void set_gc_state_all_threads(char state);
   void set_gc_state_mask(uint mask, bool value);
 
 public:
   char gc_state() const;
+  void set_gc_state_all_threads();
+  bool has_gc_state_changed() { return _gc_state_changed; }
 
   void set_concurrent_mark_in_progress(bool in_progress);
   void set_evacuation_in_progress(bool in_progress);

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

@@ -35,12 +35,23 @@
 #include "interpreter/oopMapCache.hpp"
 #include "memory/universe.hpp"
 
+bool VM_ShenandoahOperation::doit_prologue() {
+  assert(!ShenandoahHeap::heap()->has_gc_state_changed(), "GC State can only be changed on a safepoint.");
+  return true;
+}
+
+void VM_ShenandoahOperation::doit_epilogue() {
+  assert(!ShenandoahHeap::heap()->has_gc_state_changed(), "GC State was not synchronized to java threads.");
+}
+
 bool VM_ShenandoahReferenceOperation::doit_prologue() {
+  VM_ShenandoahOperation::doit_prologue();
   Heap_lock->lock();
   return true;
 }
 
 void VM_ShenandoahReferenceOperation::doit_epilogue() {
+  VM_ShenandoahOperation::doit_epilogue();
   OopMapCache::cleanup_old_entries();
   if (Universe::has_reference_pending_list()) {
     Heap_lock->notify_all();
@@ -51,34 +62,41 @@ void VM_ShenandoahReferenceOperation::doit_epilogue() {
 void VM_ShenandoahInitMark::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Init Mark", SvcGCMarker::CONCURRENT);
   _gc->entry_init_mark();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahFinalMarkStartEvac::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Final Mark", SvcGCMarker::CONCURRENT);
   _gc->entry_final_mark();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahFullGC::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Full GC", SvcGCMarker::FULL);
   _full_gc->entry_full(_gc_cause);
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahDegeneratedGC::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Degenerated GC", SvcGCMarker::CONCURRENT);
   _gc->entry_degenerated();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahInitUpdateRefs::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Init Update Refs", SvcGCMarker::CONCURRENT);
   _gc->entry_init_updaterefs();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahFinalUpdateRefs::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Final Update Refs", SvcGCMarker::CONCURRENT);
   _gc->entry_final_updaterefs();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }
 
 void VM_ShenandoahFinalRoots::doit() {
   ShenandoahGCPauseMark mark(_gc_id, "Final Roots", SvcGCMarker::CONCURRENT);
   _gc->entry_final_roots();
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
 }

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

@@ -47,14 +47,16 @@ protected:
   uint         _gc_id;
 public:
   VM_ShenandoahOperation() : _gc_id(GCId::current()) {};
-  virtual bool skip_thread_oop_barriers() const { return true; }
+  bool skip_thread_oop_barriers() const override { return true; }
+  bool doit_prologue() override;
+  void doit_epilogue() override;
 };
 
 class VM_ShenandoahReferenceOperation : public VM_ShenandoahOperation {
 public:
   VM_ShenandoahReferenceOperation() : VM_ShenandoahOperation() {};
-  bool doit_prologue();
-  void doit_epilogue();
+  bool doit_prologue() override;
+  void doit_epilogue() override;
 };
 
 class VM_ShenandoahInitMark: public VM_ShenandoahOperation {

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

@@ -620,6 +620,8 @@ void ShenandoahVerifier::verify_at_safepoint(const char *label,
   guarantee(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "only when nothing else happens");
   guarantee(ShenandoahVerify, "only when enabled, and bitmap is initialized in ShenandoahHeap::initialize");
 
+  ShenandoahHeap::heap()->set_gc_state_all_threads();
+
   // Avoid side-effect of changing workers' active thread count, but bypass concurrent/parallel protocol check
   ShenandoahPushWorkerScope verify_worker_scope(_heap->workers(), _heap->max_workers(), false /*bypass check*/);