From d445df6d150808df2ba00d716fef80d28d68e1e6 Mon Sep 17 00:00:00 2001 From: William Kemper Date: Tue, 2 Jan 2024 17:08:36 +0000 Subject: [PATCH] 8322503: Shenandoah: Clarify gc state usage Reviewed-by: ysr, gli --- src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp | 14 +++++++------- src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp | 13 ++++++++++--- .../gc/shenandoah/shenandoahThreadLocalData.hpp | 1 + .../share/gc/shenandoah/shenandoahVMOperations.cpp | 14 +++++++------- .../share/gc/shenandoah/shenandoahVerifier.cpp | 2 +- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp index 329ea941eec..71f1747a760 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -1682,7 +1682,7 @@ void ShenandoahHeap::prepare_update_heap_references(bool concurrent) { _update_refs_iterator.reset(); } -void ShenandoahHeap::set_gc_state_all_threads() { +void ShenandoahHeap::propagate_gc_state_to_java_threads() { assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint"); if (_gc_state_changed) { _gc_state_changed = false; @@ -1693,7 +1693,7 @@ void ShenandoahHeap::set_gc_state_all_threads() { } } -void ShenandoahHeap::set_gc_state_mask(uint mask, bool value) { +void ShenandoahHeap::set_gc_state(uint mask, bool value) { assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint"); _gc_state.set_cond(mask, value); _gc_state_changed = true; @@ -1701,13 +1701,13 @@ void ShenandoahHeap::set_gc_state_mask(uint mask, bool value) { void ShenandoahHeap::set_concurrent_mark_in_progress(bool in_progress) { assert(!has_forwarded_objects(), "Not expected before/after mark phase"); - set_gc_state_mask(MARKING, in_progress); + set_gc_state(MARKING, in_progress); ShenandoahBarrierSet::satb_mark_queue_set().set_active_all_threads(in_progress, !in_progress); } void ShenandoahHeap::set_evacuation_in_progress(bool in_progress) { assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Only call this at safepoint"); - set_gc_state_mask(EVACUATION, in_progress); + set_gc_state(EVACUATION, in_progress); } void ShenandoahHeap::set_concurrent_strong_root_in_progress(bool in_progress) { @@ -1719,7 +1719,7 @@ void ShenandoahHeap::set_concurrent_strong_root_in_progress(bool in_progress) { } void ShenandoahHeap::set_concurrent_weak_root_in_progress(bool cond) { - set_gc_state_mask(WEAK_ROOTS, cond); + set_gc_state(WEAK_ROOTS, cond); } GCTracer* ShenandoahHeap::tracer() { @@ -1838,7 +1838,7 @@ void ShenandoahHeap::parallel_cleaning(bool full_gc) { } void ShenandoahHeap::set_has_forwarded_objects(bool cond) { - set_gc_state_mask(HAS_FORWARDED, cond); + set_gc_state(HAS_FORWARDED, cond); } void ShenandoahHeap::set_unload_classes(bool uc) { @@ -1877,7 +1877,7 @@ void ShenandoahHeap::set_full_gc_move_in_progress(bool in_progress) { } void ShenandoahHeap::set_update_refs_in_progress(bool in_progress) { - set_gc_state_mask(UPDATEREFS, in_progress); + set_gc_state(UPDATEREFS, in_progress); } void ShenandoahHeap::register_nmethod(nmethod* nm) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp index 60db6e5776b..bc9a6eed701 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp @@ -289,12 +289,19 @@ class ShenandoahHeap : public CollectedHeap { ShenandoahSharedFlag _progress_last_gc; ShenandoahSharedFlag _concurrent_strong_root_in_progress; - void set_gc_state_mask(uint mask, bool value); + // This updates the singlular, global gc state. This must happen on a safepoint. + void set_gc_state(uint mask, bool value); public: char gc_state() const; - void set_gc_state_all_threads(); - bool has_gc_state_changed() { return _gc_state_changed; } + + // This copies the global gc state into a thread local variable for java threads. + // It is primarily intended to support quick access at barriers. + void propagate_gc_state_to_java_threads(); + + // This is public to support assertions that the state hasn't been changed off of + // a safepoint and that any changes were propagated to java threads after the safepoint. + bool has_gc_state_changed() const { return _gc_state_changed; } void set_concurrent_mark_in_progress(bool in_progress); void set_evacuation_in_progress(bool in_progress); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp b/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp index 422595e9313..9eec573cc56 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp @@ -89,6 +89,7 @@ class ShenandoahThreadLocalData { } static char gc_state(Thread* thread) { + assert(thread->is_Java_thread(), "GC state is only synchronized to java threads"); return data(thread)->_gc_state; } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp b/src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp index 970f99ad0f5..eeeb1dcad19 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp @@ -62,41 +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(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_threads(); } void VM_ShenandoahFinalMarkStartEvac::doit() { ShenandoahGCPauseMark mark(_gc_id, "Final Mark", SvcGCMarker::CONCURRENT); _gc->entry_final_mark(); - ShenandoahHeap::heap()->set_gc_state_all_threads(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_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(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_threads(); } void VM_ShenandoahDegeneratedGC::doit() { ShenandoahGCPauseMark mark(_gc_id, "Degenerated GC", SvcGCMarker::CONCURRENT); _gc->entry_degenerated(); - ShenandoahHeap::heap()->set_gc_state_all_threads(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_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(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_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(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_threads(); } void VM_ShenandoahFinalRoots::doit() { ShenandoahGCPauseMark mark(_gc_id, "Final Roots", SvcGCMarker::CONCURRENT); _gc->entry_final_roots(); - ShenandoahHeap::heap()->set_gc_state_all_threads(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_threads(); } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp index 6a977c2e4a4..f67cafdb8fe 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp @@ -620,7 +620,7 @@ 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(); + ShenandoahHeap::heap()->propagate_gc_state_to_java_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*/);