From 7e015345902c6101d0dc9dbe21a7baa098fbb820 Mon Sep 17 00:00:00 2001 From: Albert Mingkun Yang Date: Thu, 9 Mar 2023 13:40:10 +0000 Subject: [PATCH] 8303467: Serial: Refactor reference processor Reviewed-by: tschatzl, iwalulya --- .../share/gc/serial/defNewGeneration.cpp | 18 ++++++++---- .../share/gc/serial/defNewGeneration.hpp | 9 +++++- src/hotspot/share/gc/serial/genMarkSweep.cpp | 12 ++------ src/hotspot/share/gc/serial/genMarkSweep.hpp | 2 +- src/hotspot/share/gc/serial/markSweep.cpp | 19 ++++++------- src/hotspot/share/gc/serial/markSweep.hpp | 5 ++-- .../share/gc/serial/tenuredGeneration.cpp | 7 +---- .../share/gc/shared/genCollectedHeap.cpp | 28 ++----------------- .../share/gc/shared/genCollectedHeap.hpp | 3 -- src/hotspot/share/gc/shared/generation.cpp | 12 +------- src/hotspot/share/gc/shared/generation.hpp | 13 --------- .../share/gc/shared/referenceProcessor.cpp | 8 ++---- .../share/gc/shared/referenceProcessor.hpp | 27 +----------------- 13 files changed, 43 insertions(+), 120 deletions(-) diff --git a/src/hotspot/share/gc/serial/defNewGeneration.cpp b/src/hotspot/share/gc/serial/defNewGeneration.cpp index bfd01aac8d9..31ca98f9570 100644 --- a/src/hotspot/share/gc/serial/defNewGeneration.cpp +++ b/src/hotspot/share/gc/serial/defNewGeneration.cpp @@ -372,6 +372,8 @@ DefNewGeneration::DefNewGeneration(ReservedSpace rs, _tenuring_threshold = MaxTenuringThreshold; _pretenure_size_threshold_words = PretenureSizeThreshold >> LogHeapWordSize; + _ref_processor = nullptr; + _gc_timer = new STWGCTimer(); _gc_tracer = new DefNewTracer(); @@ -615,6 +617,12 @@ void DefNewGeneration::compute_new_size() { } } +void DefNewGeneration::ref_processor_init() { + assert(_ref_processor == nullptr, "a reference processor already exists"); + assert(!_reserved.is_empty(), "empty generation?"); + _span_based_discoverer.set_span(_reserved); + _ref_processor = new ReferenceProcessor(&_span_based_discoverer); // a vanilla reference processor +} size_t DefNewGeneration::capacity() const { return eden()->capacity() @@ -720,11 +728,6 @@ void DefNewGeneration::collect(bool full, SerialHeap* heap = SerialHeap::heap(); - _gc_timer->register_gc_start(); - _gc_tracer->report_gc_start(heap->gc_cause(), _gc_timer->gc_start()); - - _old_gen = heap->old_gen(); - // If the next generation is too full to accommodate promotion // from this generation, pass on collection; let the next generation // do it. @@ -734,6 +737,11 @@ void DefNewGeneration::collect(bool full, return; } assert(to()->is_empty(), "Else not collection_attempt_is_safe"); + _gc_timer->register_gc_start(); + _gc_tracer->report_gc_start(heap->gc_cause(), _gc_timer->gc_start()); + _ref_processor->start_discovery(clear_all_soft_refs); + + _old_gen = heap->old_gen(); init_assuming_no_promotion_failure(); diff --git a/src/hotspot/share/gc/serial/defNewGeneration.hpp b/src/hotspot/share/gc/serial/defNewGeneration.hpp index b56c0679d30..03ee369fcd1 100644 --- a/src/hotspot/share/gc/serial/defNewGeneration.hpp +++ b/src/hotspot/share/gc/serial/defNewGeneration.hpp @@ -52,13 +52,16 @@ class STWGCTimer; class DefNewGeneration: public Generation { friend class VMStructs; -protected: Generation* _old_gen; uint _tenuring_threshold; // Tenuring threshold for next collection. AgeTable _age_table; // Size of object to pretenure in words; command line provides bytes size_t _pretenure_size_threshold_words; + // ("Weak") Reference processing support + SpanSubjectToDiscoveryClosure _span_based_discoverer; + ReferenceProcessor* _ref_processor; + AgeTable* age_table() { return &_age_table; } // Initialize state to optimistically assume no promotion failure will @@ -162,6 +165,10 @@ protected: virtual Generation::Name kind() { return Generation::DefNew; } + // allocate and initialize ("weak") refs processing support + void ref_processor_init(); + ReferenceProcessor* const ref_processor() { return _ref_processor; } + // Accessing spaces ContiguousSpace* eden() const { return _eden_space; } ContiguousSpace* from() const { return _from_space; } diff --git a/src/hotspot/share/gc/serial/genMarkSweep.cpp b/src/hotspot/share/gc/serial/genMarkSweep.cpp index afdfa3c5fba..7fce5a142b3 100644 --- a/src/hotspot/share/gc/serial/genMarkSweep.cpp +++ b/src/hotspot/share/gc/serial/genMarkSweep.cpp @@ -63,7 +63,7 @@ #include "jvmci/jvmci.hpp" #endif -void GenMarkSweep::invoke_at_safepoint(ReferenceProcessor* rp, bool clear_all_softrefs) { +void GenMarkSweep::invoke_at_safepoint(bool clear_all_softrefs) { assert(SafepointSynchronize::is_at_safepoint(), "must be at a safepoint"); GenCollectedHeap* gch = GenCollectedHeap::heap(); @@ -73,11 +73,6 @@ void GenMarkSweep::invoke_at_safepoint(ReferenceProcessor* rp, bool clear_all_so } #endif - // hook up weak ref data so it can be used during Mark-Sweep - assert(ref_processor() == nullptr, "no stomping"); - assert(rp != nullptr, "should be non-null"); - set_ref_processor(rp); - gch->trace_heap_before_gc(_gc_tracer); // Increment the invocation count @@ -133,9 +128,6 @@ void GenMarkSweep::invoke_at_safepoint(ReferenceProcessor* rp, bool clear_all_so gch->prune_scavengable_nmethods(); - // refs processing: clean slate - set_ref_processor(nullptr); - // Update heap occupancy information which is used as // input to soft ref clearing policy at the next gc. Universe::heap()->update_capacity_and_used_at_gc(); @@ -182,6 +174,8 @@ void GenMarkSweep::mark_sweep_phase1(bool clear_all_softrefs) { ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_stw_fullgc_mark); + ref_processor()->start_discovery(clear_all_softrefs); + { StrongRootsScope srs(0); diff --git a/src/hotspot/share/gc/serial/genMarkSweep.hpp b/src/hotspot/share/gc/serial/genMarkSweep.hpp index f46a7638a6a..be830318bd0 100644 --- a/src/hotspot/share/gc/serial/genMarkSweep.hpp +++ b/src/hotspot/share/gc/serial/genMarkSweep.hpp @@ -29,7 +29,7 @@ class GenMarkSweep : public MarkSweep { public: - static void invoke_at_safepoint(ReferenceProcessor* rp, bool clear_all_softrefs); + static void invoke_at_safepoint(bool clear_all_softrefs); private: diff --git a/src/hotspot/share/gc/serial/markSweep.cpp b/src/hotspot/share/gc/serial/markSweep.cpp index 6a6e2796af6..d1dc1c07d8e 100644 --- a/src/hotspot/share/gc/serial/markSweep.cpp +++ b/src/hotspot/share/gc/serial/markSweep.cpp @@ -33,15 +33,10 @@ #include "memory/universe.hpp" #include "oops/access.inline.hpp" #include "oops/compressedOops.inline.hpp" -#include "oops/instanceClassLoaderKlass.inline.hpp" -#include "oops/instanceKlass.inline.hpp" -#include "oops/instanceMirrorKlass.inline.hpp" -#include "oops/instanceRefKlass.inline.hpp" #include "oops/methodData.hpp" #include "oops/objArrayKlass.inline.hpp" #include "oops/oop.inline.hpp" #include "oops/typeArrayOop.inline.hpp" -#include "utilities/macros.hpp" #include "utilities/stack.inline.hpp" uint MarkSweep::_total_invocations = 0; @@ -53,10 +48,12 @@ Stack MarkSweep::_preserved_overflow_stack; size_t MarkSweep::_preserved_count = 0; size_t MarkSweep::_preserved_count_max = 0; PreservedMark* MarkSweep::_preserved_marks = nullptr; -ReferenceProcessor* MarkSweep::_ref_processor = nullptr; STWGCTimer* MarkSweep::_gc_timer = nullptr; SerialOldTracer* MarkSweep::_gc_tracer = nullptr; +AlwaysTrueClosure MarkSweep::_always_true_closure; +ReferenceProcessor* MarkSweep::_ref_processor; + StringDedup::Requests* MarkSweep::_string_dedup_requests = nullptr; MarkSweep::FollowRootClosure MarkSweep::follow_root_closure; @@ -168,11 +165,6 @@ void MarkSweep::preserve_mark(oop obj, markWord mark) { } } -void MarkSweep::set_ref_processor(ReferenceProcessor* rp) { - _ref_processor = rp; - mark_and_push_closure.set_ref_discoverer(_ref_processor); -} - void MarkSweep::mark_object(oop obj) { if (StringDedup::is_enabled() && java_lang_String::is_instance(obj) && @@ -252,4 +244,9 @@ void MarkSweep::initialize() { MarkSweep::_gc_timer = new STWGCTimer(); MarkSweep::_gc_tracer = new SerialOldTracer(); MarkSweep::_string_dedup_requests = new StringDedup::Requests(); + + // The Full GC operates on the entire heap so all objects should be subject + // to discovery, hence the _always_true_closure. + MarkSweep::_ref_processor = new ReferenceProcessor(&_always_true_closure); + mark_and_push_closure.set_ref_discoverer(_ref_processor); } diff --git a/src/hotspot/share/gc/serial/markSweep.hpp b/src/hotspot/share/gc/serial/markSweep.hpp index 92db32da431..60360c86d50 100644 --- a/src/hotspot/share/gc/serial/markSweep.hpp +++ b/src/hotspot/share/gc/serial/markSweep.hpp @@ -26,6 +26,7 @@ #define SHARE_GC_SERIAL_MARKSWEEP_HPP #include "gc/shared/collectedHeap.hpp" +#include "gc/shared/referenceProcessor.hpp" #include "gc/shared/stringdedup/stringDedup.hpp" #include "gc/shared/taskqueue.hpp" #include "memory/iterator.hpp" @@ -35,7 +36,6 @@ #include "utilities/growableArray.hpp" #include "utilities/stack.hpp" -class ReferenceProcessor; class DataLayout; class SerialOldTracer; class STWGCTimer; @@ -104,7 +104,7 @@ class MarkSweep : AllStatic { static size_t _preserved_count_max; static PreservedMark* _preserved_marks; - // Reference processing (used in ...follow_contents) + static AlwaysTrueClosure _always_true_closure; static ReferenceProcessor* _ref_processor; static STWGCTimer* _gc_timer; @@ -132,7 +132,6 @@ class MarkSweep : AllStatic { // Reference Processing static ReferenceProcessor* const ref_processor() { return _ref_processor; } - static void set_ref_processor(ReferenceProcessor* rp); static STWGCTimer* gc_timer() { return _gc_timer; } static SerialOldTracer* gc_tracer() { return _gc_tracer; } diff --git a/src/hotspot/share/gc/serial/tenuredGeneration.cpp b/src/hotspot/share/gc/serial/tenuredGeneration.cpp index d84ee8cc09f..70674461135 100644 --- a/src/hotspot/share/gc/serial/tenuredGeneration.cpp +++ b/src/hotspot/share/gc/serial/tenuredGeneration.cpp @@ -437,11 +437,6 @@ void TenuredGeneration::collect(bool full, bool is_tlab) { GenCollectedHeap* gch = GenCollectedHeap::heap(); - // Temporarily expand the span of our ref processor, so - // refs discovery is over the entire heap, not just this generation - ReferenceProcessorSpanMutator - x(ref_processor(), gch->reserved_region()); - STWGCTimer* gc_timer = GenMarkSweep::gc_timer(); gc_timer->register_gc_start(); @@ -450,7 +445,7 @@ void TenuredGeneration::collect(bool full, gch->pre_full_gc_dump(gc_timer); - GenMarkSweep::invoke_at_safepoint(ref_processor(), clear_all_soft_refs); + GenMarkSweep::invoke_at_safepoint(clear_all_soft_refs); gch->post_full_gc_dump(gc_timer); diff --git a/src/hotspot/share/gc/shared/genCollectedHeap.cpp b/src/hotspot/share/gc/shared/genCollectedHeap.cpp index 6faada2e42a..56cc45e2e80 100644 --- a/src/hotspot/share/gc/shared/genCollectedHeap.cpp +++ b/src/hotspot/share/gc/shared/genCollectedHeap.cpp @@ -189,10 +189,11 @@ static GenIsScavengable _is_scavengable; void GenCollectedHeap::post_initialize() { CollectedHeap::post_initialize(); - ref_processing_init(); DefNewGeneration* def_new_gen = (DefNewGeneration*)_young_gen; + def_new_gen->ref_processor_init(); + initialize_size_policy(def_new_gen->eden()->capacity(), _old_gen->capacity(), def_new_gen->from()->capacity()); @@ -202,11 +203,6 @@ void GenCollectedHeap::post_initialize() { ScavengableNMethods::initialize(&_is_scavengable); } -void GenCollectedHeap::ref_processing_init() { - _young_gen->ref_processor_init(); - _old_gen->ref_processor_init(); -} - PreGenGCValues GenCollectedHeap::get_pre_gc_values() const { const DefNewGeneration* const def_new_gen = (DefNewGeneration*) young_gen(); @@ -455,29 +451,9 @@ void GenCollectedHeap::collect_generation(Generation* gen, bool full, size_t siz // Do collection work { - // Note on ref discovery: For what appear to be historical reasons, - // GCH enables and disabled (by enqueuing) refs discovery. - // In the future this should be moved into the generation's - // collect method so that ref discovery and enqueueing concerns - // are local to a generation. The collect method could return - // an appropriate indication in the case that notification on - // the ref lock was needed. This will make the treatment of - // weak refs more uniform (and indeed remove such concerns - // from GCH). XXX - save_marks(); // save marks for all gens - // We want to discover references, but not process them yet. - // This mode is disabled in process_discovered_references if the - // generation does some collection work, or in - // enqueue_discovered_references if the generation returns - // without doing any work. - ReferenceProcessor* rp = gen->ref_processor(); - rp->start_discovery(clear_soft_refs); gen->collect(full, clear_soft_refs, size, is_tlab); - - rp->disable_discovery(); - rp->verify_no_references_recorded(); } COMPILER2_OR_JVMCI_PRESENT(DerivedPointerTable::update_pointers()); diff --git a/src/hotspot/share/gc/shared/genCollectedHeap.hpp b/src/hotspot/share/gc/shared/genCollectedHeap.hpp index e7b98427880..2ab3da069f7 100644 --- a/src/hotspot/share/gc/shared/genCollectedHeap.hpp +++ b/src/hotspot/share/gc/shared/genCollectedHeap.hpp @@ -94,9 +94,6 @@ private: // Reserve aligned space for the heap as needed by the contained generations. ReservedHeapSpace allocate(size_t alignment); - // Initialize ("weak") refs processing support - void ref_processing_init(); - PreGenGCValues get_pre_gc_values() const; protected: diff --git a/src/hotspot/share/gc/shared/generation.cpp b/src/hotspot/share/gc/shared/generation.cpp index 55881e20841..b09b609e8d4 100644 --- a/src/hotspot/share/gc/shared/generation.cpp +++ b/src/hotspot/share/gc/shared/generation.cpp @@ -42,8 +42,7 @@ #include "utilities/events.hpp" Generation::Generation(ReservedSpace rs, size_t initial_size) : - _gc_manager(nullptr), - _ref_processor(nullptr) { + _gc_manager(nullptr) { if (!_virtual_space.initialize(rs, initial_size)) { vm_exit_during_initialization("Could not reserve enough space for " "object heap"); @@ -70,15 +69,6 @@ size_t Generation::max_capacity() const { return reserved().byte_size(); } -// By default we get a single threaded default reference processor; -// generations needing multi-threaded refs processing or discovery override this method. -void Generation::ref_processor_init() { - assert(_ref_processor == nullptr, "a reference processor already exists"); - assert(!_reserved.is_empty(), "empty generation?"); - _span_based_discoverer.set_span(_reserved); - _ref_processor = new ReferenceProcessor(&_span_based_discoverer); // a vanilla reference processor -} - void Generation::print() const { print_on(tty); } void Generation::print_on(outputStream* st) const { diff --git a/src/hotspot/share/gc/shared/generation.hpp b/src/hotspot/share/gc/shared/generation.hpp index 3bbbd397179..cfb64b17354 100644 --- a/src/hotspot/share/gc/shared/generation.hpp +++ b/src/hotspot/share/gc/shared/generation.hpp @@ -85,10 +85,6 @@ class Generation: public CHeapObj { // Memory area reserved for generation VirtualSpace _virtual_space; - // ("Weak") Reference processing support - SpanSubjectToDiscoveryClosure _span_based_discoverer; - ReferenceProcessor* _ref_processor; - // Performance Counters CollectorCounters* _gc_counters; @@ -115,12 +111,6 @@ class Generation: public CHeapObj { GenGrain = 1 << LogOfGenGrain }; - // allocate and initialize ("weak") refs processing support - void ref_processor_init(); - void set_ref_processor(ReferenceProcessor* rp) { - assert(_ref_processor == nullptr, "clobbering existing _ref_processor"); - _ref_processor = rp; - } virtual Generation::Name kind() { return Generation::Other; } @@ -361,9 +351,6 @@ class Generation: public CHeapObj { virtual const char* name() const = 0; virtual const char* short_name() const = 0; - // Reference Processing accessor - ReferenceProcessor* const ref_processor() { return _ref_processor; } - // Iteration. // Iterate over all the ref-containing fields of all objects in the diff --git a/src/hotspot/share/gc/shared/referenceProcessor.cpp b/src/hotspot/share/gc/shared/referenceProcessor.cpp index 8c0f227839c..d090c2d2b32 100644 --- a/src/hotspot/share/gc/shared/referenceProcessor.cpp +++ b/src/hotspot/share/gc/shared/referenceProcessor.cpp @@ -72,15 +72,13 @@ void ReferenceProcessor::init_statics() { "Unrecognized RefDiscoveryPolicy"); } -void ReferenceProcessor::enable_discovery(bool check_no_refs) { +void ReferenceProcessor::enable_discovery() { #ifdef ASSERT // Verify that we're not currently discovering refs assert(!_discovering_refs, "nested call?"); - if (check_no_refs) { - // Verify that the discovered lists are empty - verify_no_references_recorded(); - } + // Verify that the discovered lists are empty + verify_no_references_recorded(); #endif // ASSERT _discovering_refs = true; diff --git a/src/hotspot/share/gc/shared/referenceProcessor.hpp b/src/hotspot/share/gc/shared/referenceProcessor.hpp index 77f095a13fb..99f44904b9e 100644 --- a/src/hotspot/share/gc/shared/referenceProcessor.hpp +++ b/src/hotspot/share/gc/shared/referenceProcessor.hpp @@ -400,7 +400,7 @@ public: void set_is_subject_to_discovery_closure(BoolObjectClosure* cl) { _is_subject_to_discovery = cl; } // start and stop weak ref discovery - void enable_discovery(bool check_no_refs = true); + void enable_discovery(); void disable_discovery() { _discovering_refs = false; } bool discovery_enabled() { return _discovering_refs; } @@ -445,9 +445,6 @@ class SpanSubjectToDiscoveryClosure : public BoolObjectClosure { public: SpanSubjectToDiscoveryClosure() : BoolObjectClosure(), _span() { } - SpanSubjectToDiscoveryClosure(MemRegion span) : BoolObjectClosure(), _span(span) { } - - MemRegion span() const { return _span; } void set_span(MemRegion mr) { _span = mr; @@ -476,28 +473,6 @@ public: } }; -// A utility class to temporarily mutate the span of the -// given ReferenceProcessor in the scope that contains it. -class ReferenceProcessorSpanMutator : StackObj { - ReferenceProcessor* _rp; - SpanSubjectToDiscoveryClosure _discoverer; - BoolObjectClosure* _old_discoverer; - -public: - ReferenceProcessorSpanMutator(ReferenceProcessor* rp, - MemRegion span): - _rp(rp), - _discoverer(span), - _old_discoverer(rp->is_subject_to_discovery_closure()) { - - rp->set_is_subject_to_discovery_closure(&_discoverer); - } - - ~ReferenceProcessorSpanMutator() { - _rp->set_is_subject_to_discovery_closure(_old_discoverer); - } -}; - // A utility class to temporarily change the MT'ness of // reference discovery for the given ReferenceProcessor // in the scope that contains it.