8330145: Serial: Refactor SerialHeap::scan_evacuated_objs

Reviewed-by: gli, tschatzl

Author: albertnetymk

src/hotspot/share/gc/serial/cardTableRS.cpp

@@ -31,12 +31,11 @@
 #include "memory/iterator.inline.hpp"
 #include "utilities/align.hpp"
 
-void CardTableRS::scan_old_to_young_refs(TenuredGeneration* tg, HeapWord* saved_mark_word) {
+void CardTableRS::scan_old_to_young_refs(TenuredGeneration* tg, HeapWord* saved_top) {
   const MemRegion ur    = tg->used_region();
-  const MemRegion urasm = MemRegion(tg->space()->bottom(), saved_mark_word);
+  const MemRegion urasm = MemRegion(tg->space()->bottom(), saved_top);
 
   assert(ur.contains(urasm),
-         "Did you forget to call save_marks()? "
          "[" PTR_FORMAT ", " PTR_FORMAT ") is not contained in "
          "[" PTR_FORMAT ", " PTR_FORMAT ")",
          p2i(urasm.start()), p2i(urasm.end()), p2i(ur.start()), p2i(ur.end()));

src/hotspot/share/gc/serial/cardTableRS.hpp

@@ -60,7 +60,7 @@ class CardTableRS : public CardTable {
 public:
   CardTableRS(MemRegion whole_heap);
 
-  void scan_old_to_young_refs(TenuredGeneration* tg, HeapWord* saved_mark_word);
+  void scan_old_to_young_refs(TenuredGeneration* tg, HeapWord* saved_top);
 
   void inline_write_ref_field_gc(void* field) {
     CardValue* byte = byte_for(field);

src/hotspot/share/gc/serial/defNewGeneration.cpp

@@ -24,7 +24,6 @@
 
 #include "precompiled.hpp"
 #include "gc/serial/cardTableRS.hpp"
-#include "gc/serial/defNewGeneration.inline.hpp"
 #include "gc/serial/serialGcRefProcProxyTask.hpp"
 #include "gc/serial/serialHeap.inline.hpp"
 #include "gc/serial/serialStringDedup.inline.hpp"
@@ -678,19 +677,13 @@ void DefNewGeneration::collect(bool   full,
   // The preserved marks should be empty at the start of the GC.
   _preserved_marks_set.init(1);
 
-  assert(heap->no_allocs_since_save_marks(),
-         "save marks have not been newly set.");
-
   YoungGenScanClosure young_gen_cl(this);
   OldGenScanClosure   old_gen_cl(this);
 
   FastEvacuateFollowersClosure evacuate_followers(heap,
                                                   &young_gen_cl,
                                                   &old_gen_cl);
 
-  assert(heap->no_allocs_since_save_marks(),
-         "save marks have not been newly set.");
-
   {
     StrongRootsScope srs(0);
     RootScanClosure root_cl{this};
@@ -700,13 +693,14 @@ void DefNewGeneration::collect(bool   full,
                                   NMethodToOopClosure::FixRelocations,
                                   false /* keepalive_nmethods */);
 
+    HeapWord* saved_top_in_old_gen = _old_gen->space()->top();
     heap->process_roots(SerialHeap::SO_ScavengeCodeCache,
                         &root_cl,
                         &cld_cl,
                         &cld_cl,
                         &code_cl);
 
-    _old_gen->scan_old_to_young_refs();
+    _old_gen->scan_old_to_young_refs(saved_top_in_old_gen);
   }
 
   // "evacuate followers".
@@ -723,16 +717,12 @@ void DefNewGeneration::collect(bool   full,
     _gc_tracer->report_tenuring_threshold(tenuring_threshold());
     pt.print_all_references();
   }
-  assert(heap->no_allocs_since_save_marks(), "save marks have not been newly set.");
 
   {
     AdjustWeakRootClosure cl{this};
     WeakProcessor::weak_oops_do(&is_alive, &cl);
   }
 
-  // Verify that the usage of keep_alive didn't copy any objects.
-  assert(heap->no_allocs_since_save_marks(), "save marks have not been newly set.");
-
   _string_dedup_requests.flush();
 
   if (!_promotion_failed) {
@@ -892,15 +882,6 @@ void DefNewGeneration::drain_promo_failure_scan_stack() {
   }
 }
 
-void DefNewGeneration::save_marks() {
-  set_saved_mark_word();
-}
-
-
-bool DefNewGeneration::no_allocs_since_save_marks() {
-  return saved_mark_at_top();
-}
-
 void DefNewGeneration::contribute_scratch(void*& scratch, size_t& num_words) {
   if (_promotion_failed) {
     return;

src/hotspot/share/gc/serial/defNewGeneration.hpp

@@ -171,10 +171,6 @@ class DefNewGeneration: public Generation {
   ContiguousSpace* from() const           { return _from_space; }
   ContiguousSpace* to()   const           { return _to_space;   }
 
-  HeapWord* saved_mark_word()   const    { return _saved_mark_word; }
-  void set_saved_mark_word()             { _saved_mark_word = to()->top(); }
-  bool saved_mark_at_top()               { return _saved_mark_word == _to_space->top(); }
-
   // Space enquiries
   size_t capacity() const;
   size_t used() const;
@@ -243,17 +239,6 @@ class DefNewGeneration: public Generation {
   // Save the tops for eden, from, and to
   void record_spaces_top();
 
-  // Accessing marks
-  void save_marks();
-
-  bool no_allocs_since_save_marks();
-
-  // Need to declare the full complement of closures, whether we'll
-  // override them or not, or get message from the compiler:
-  //   oop_since_save_marks_iterate_nv hides virtual function...
-  template <typename OopClosureType>
-  void oop_since_save_marks_iterate(OopClosureType* cl);
-
   // For Old collection (part of running Full GC), the DefNewGeneration can
   // contribute the free part of "to-space" as the scratch space.
   void contribute_scratch(void*& scratch, size_t& num_words);

src/hotspot/share/gc/serial/defNewGeneration.inline.hpp

@@ -200,32 +200,6 @@ class Generation: public CHeapObj<mtGC> {
   void set_gc_manager(GCMemoryManager* gc_manager) {
     _gc_manager = gc_manager;
   }
-
-  // Apply "blk->do_oop" to the addresses of all reference fields in objects
-  // starting with the _saved_mark_word, which was noted during a generation's
-  // save_marks and is required to denote the head of an object.
-  // Fields in objects allocated by applications of the closure
-  // *are* included in the iteration.
-  // Updates saved_mark_word to point to just after the last object iterated over.
-  template <typename OopClosureType>
-  void oop_since_save_marks_iterate_impl(OopClosureType* blk, ContiguousSpace* space, HeapWord* saved_mark_word);
 };
 
-template <typename OopClosureType>
-void Generation::oop_since_save_marks_iterate_impl(OopClosureType* blk, ContiguousSpace* space, HeapWord* saved_mark_word) {
-  HeapWord* t;
-  HeapWord* p = saved_mark_word;
-  assert(p != nullptr, "expected saved mark");
-
-  const intx interval = PrefetchScanIntervalInBytes;
-  do {
-    t = space->top();
-    while (p < t) {
-      Prefetch::write(p, interval);
-      oop m = cast_to_oop(p);
-      p += m->oop_iterate_size(blk);
-    }
-  } while (t < space->top());
-}
-
 #endif // SHARE_GC_SERIAL_GENERATION_HPP

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

@@ -748,10 +748,6 @@ void SerialFullGC::invoke_at_safepoint(bool clear_all_softrefs) {
 
   restore_marks();
 
-  // Set saved marks for allocation profiler (and other things? -- dld)
-  // (Should this be in general part?)
-  gch->save_marks();
-
   deallocate_stacks();
 
   SerialFullGC::_string_dedup_requests->flush();

src/hotspot/share/gc/serial/serialFullGC.cpp

@@ -30,7 +30,6 @@
 #include "code/codeCache.hpp"
 #include "compiler/oopMap.hpp"
 #include "gc/serial/cardTableRS.hpp"
-#include "gc/serial/defNewGeneration.inline.hpp"
 #include "gc/serial/serialFullGC.hpp"
 #include "gc/serial/serialHeap.inline.hpp"
 #include "gc/serial/serialMemoryPools.hpp"
@@ -757,17 +756,33 @@ void SerialHeap::process_roots(ScanningOption so,
   }
 }
 
-bool SerialHeap::no_allocs_since_save_marks() {
-  return _young_gen->no_allocs_since_save_marks() &&
-         _old_gen->no_allocs_since_save_marks();
+template <typename OopClosureType>
+static void oop_iterate_from(OopClosureType* blk, ContiguousSpace* space, HeapWord** from) {
+  assert(*from != nullptr, "precondition");
+  HeapWord* t;
+  HeapWord* p = *from;
+
+  const intx interval = PrefetchScanIntervalInBytes;
+  do {
+    t = space->top();
+    while (p < t) {
+      Prefetch::write(p, interval);
+      p += cast_to_oop(p)->oop_iterate_size(blk);
+    }
+  } while (t < space->top());
+
+  *from = space->top();
 }
 
 void SerialHeap::scan_evacuated_objs(YoungGenScanClosure* young_cl,
                                      OldGenScanClosure* old_cl) {
+  ContiguousSpace* to_space = young_gen()->to();
   do {
-    young_gen()->oop_since_save_marks_iterate(young_cl);
-    old_gen()->oop_since_save_marks_iterate(old_cl);
-  } while (!no_allocs_since_save_marks());
+    oop_iterate_from(young_cl, to_space, &_young_gen_saved_top);
+    oop_iterate_from(old_cl, old_gen()->space(), &_old_gen_saved_top);
+    // Recheck to-space only, because postcondition of oop_iterate_from is no
+    // unscanned objs
+  } while (_young_gen_saved_top != to_space->top());
   guarantee(young_gen()->promo_failure_scan_is_complete(), "Failed to finish scan");
 }
 
@@ -934,8 +949,8 @@ bool SerialHeap::is_maximal_no_gc() const {
 }
 
 void SerialHeap::save_marks() {
-  _young_gen->save_marks();
-  _old_gen->save_marks();
+  _young_gen_saved_top = _young_gen->to()->top();
+  _old_gen_saved_top = _old_gen->space()->top();
 }
 
 void SerialHeap::verify(VerifyOption option /* ignored */) {

src/hotspot/share/gc/serial/serialHeap.cpp

@@ -85,7 +85,8 @@ class SerialHeap : public CollectedHeap {
 private:
   DefNewGeneration* _young_gen;
   TenuredGeneration* _old_gen;
-
+  HeapWord* _young_gen_saved_top;
+  HeapWord* _old_gen_saved_top;
 private:
   // The singleton CardTable Remembered Set.
   CardTableRS* _rem_set;
@@ -279,10 +280,6 @@ class SerialHeap : public CollectedHeap {
   // in other generations, it should call this method.
   void save_marks();
 
-  // Returns "true" iff no allocations have occurred since the last
-  // call to "save_marks".
-  bool no_allocs_since_save_marks();
-
   // Returns true if an incremental collection is likely to fail.
   // We optionally consult the young gen, if asked to do so;
   // otherwise we base our answer on whether the previous incremental

src/hotspot/share/gc/serial/serialHeap.hpp

@@ -27,7 +27,6 @@
 
 #include "gc/serial/serialHeap.hpp"
 
-#include "gc/serial/defNewGeneration.inline.hpp"
 #include "gc/serial/tenuredGeneration.inline.hpp"
 
 class ScavengeHelper {

src/hotspot/share/gc/serial/serialHeap.inline.hpp

@@ -280,8 +280,8 @@ HeapWord* TenuredGeneration::block_start(const void* addr) const {
   }
 }
 
-void TenuredGeneration::scan_old_to_young_refs() {
-  _rs->scan_old_to_young_refs(this, saved_mark_word());
+void TenuredGeneration::scan_old_to_young_refs(HeapWord* saved_top_in_old_gen) {
+  _rs->scan_old_to_young_refs(this, saved_top_in_old_gen);
 }
 
 TenuredGeneration::TenuredGeneration(ReservedSpace rs,
@@ -504,14 +504,6 @@ void TenuredGeneration::complete_loaded_archive_space(MemRegion archive_space) {
   }
 }
 
-void TenuredGeneration::save_marks() {
-  set_saved_mark_word();
-}
-
-bool TenuredGeneration::no_allocs_since_save_marks() {
-  return saved_mark_at_top();
-}
-
 void TenuredGeneration::gc_epilogue() {
   // update the generation and space performance counters
   update_counters();

src/hotspot/share/gc/serial/tenuredGeneration.cpp

@@ -90,8 +90,6 @@ class TenuredGeneration: public Generation {
 
   TenuredSpace* space() const { return _the_space; }
   HeapWord* saved_mark_word() const { return _saved_mark_word; }
-  void set_saved_mark_word() { _saved_mark_word = _the_space->top(); }
-  bool saved_mark_at_top() { return _saved_mark_word == space()->top(); }
 
   // Grow generation with specified size (returns false if unable to grow)
   bool grow_by(size_t bytes);
@@ -114,7 +112,7 @@ class TenuredGeneration: public Generation {
 
   HeapWord* block_start(const void* addr) const;
 
-  void scan_old_to_young_refs();
+  void scan_old_to_young_refs(HeapWord* saved_top_in_old_gen);
 
   bool is_in(const void* p) const;
 
@@ -139,13 +137,6 @@ class TenuredGeneration: public Generation {
   virtual inline HeapWord* allocate(size_t word_size, bool is_tlab);
   virtual inline HeapWord* par_allocate(size_t word_size, bool is_tlab);
 
-  template <typename OopClosureType>
-  void oop_since_save_marks_iterate(OopClosureType* cl);
-
-  void save_marks();
-
-  bool no_allocs_since_save_marks();
-
   virtual void collect(bool full,
                        bool clear_all_soft_refs,
                        size_t size,

src/hotspot/share/gc/serial/tenuredGeneration.hpp

@@ -61,10 +61,4 @@ HeapWord* TenuredGeneration::par_allocate(size_t word_size,
   return _the_space->par_allocate(word_size);
 }
 
-template <typename OopClosureType>
-void TenuredGeneration::oop_since_save_marks_iterate(OopClosureType* blk) {
-  Generation::oop_since_save_marks_iterate_impl(blk, _the_space, _saved_mark_word);
-  set_saved_mark_word();
-}
-
 #endif // SHARE_GC_SERIAL_TENUREDGENERATION_INLINE_HPP