8303467: Serial: Refactor reference processor

Reviewed-by: tschatzl, iwalulya

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();
 

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; }

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);
 

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:
 

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<PreservedMark, mtGC>    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);
 }

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; }

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);
 

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());

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:

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 {

src/hotspot/share/gc/shared/generation.hpp

@@ -85,10 +85,6 @@ class Generation: public CHeapObj<mtGC> {
   // 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<mtGC> {
     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<mtGC> {
   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

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;

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.