8322383: G1: Only preserve marks on objects that are actually moved

Reviewed-by: ayang, tschatzl

src/hotspot/share/gc/g1/g1FullCollector.cpp

@@ -120,8 +120,8 @@ G1FullCollector::G1FullCollector(G1CollectedHeap* heap,
     _oop_queue_set(_num_workers),
     _array_queue_set(_num_workers),
     _preserved_marks_set(true),
-    _serial_compaction_point(this),
-    _humongous_compaction_point(this),
+    _serial_compaction_point(this, nullptr),
+    _humongous_compaction_point(this, nullptr),
     _is_alive(this, heap->concurrent_mark()->mark_bitmap()),
     _is_alive_mutator(heap->ref_processor_stw(), &_is_alive),
     _humongous_compaction_regions(8),
@@ -142,11 +142,13 @@ G1FullCollector::G1FullCollector(G1CollectedHeap* heap,
   }
 
   for (uint i = 0; i < _num_workers; i++) {
-    _markers[i] = new G1FullGCMarker(this, i, _preserved_marks_set.get(i), _live_stats);
-    _compaction_points[i] = new G1FullGCCompactionPoint(this);
+    _markers[i] = new G1FullGCMarker(this, i, _live_stats);
+    _compaction_points[i] = new G1FullGCCompactionPoint(this, _preserved_marks_set.get(i));
     _oop_queue_set.register_queue(i, marker(i)->oop_stack());
     _array_queue_set.register_queue(i, marker(i)->objarray_stack());
   }
+  _serial_compaction_point.set_preserved_stack(_preserved_marks_set.get(0));
+  _humongous_compaction_point.set_preserved_stack(_preserved_marks_set.get(0));
   _region_attr_table.initialize(heap->reserved(), HeapRegion::GrainBytes);
 }
 

src/hotspot/share/gc/g1/g1FullGCAdjustTask.cpp

@@ -91,8 +91,8 @@ void G1FullGCAdjustTask::work(uint worker_id) {
   ResourceMark rm;
 
   // Adjust preserved marks first since they are not balanced.
-  G1FullGCMarker* marker = collector()->marker(worker_id);
-  marker->preserved_stack()->adjust_during_full_gc();
+  G1FullGCCompactionPoint* cp = collector()->compaction_point(worker_id);
+  cp->preserved_stack()->adjust_during_full_gc();
 
   {
     // Adjust the weak roots.

src/hotspot/share/gc/g1/g1FullGCCompactionPoint.cpp

@@ -30,10 +30,11 @@
 #include "oops/oop.inline.hpp"
 #include "utilities/debug.hpp"
 
-G1FullGCCompactionPoint::G1FullGCCompactionPoint(G1FullCollector* collector) :
+G1FullGCCompactionPoint::G1FullGCCompactionPoint(G1FullCollector* collector, PreservedMarks* preserved_stack) :
     _collector(collector),
     _current_region(nullptr),
-    _compaction_top(nullptr) {
+    _compaction_top(nullptr),
+    _preserved_stack(preserved_stack) {
   _compaction_regions = new (mtGC) GrowableArray<HeapRegion*>(32, mtGC);
   _compaction_region_iterator = _compaction_regions->begin();
 }
@@ -102,6 +103,9 @@ void G1FullGCCompactionPoint::forward(oop object, size_t size) {
 
   // Store a forwarding pointer if the object should be moved.
   if (cast_from_oop<HeapWord*>(object) != _compaction_top) {
+    if (!object->is_forwarded()) {
+      preserved_stack()->push_if_necessary(object, object->mark());
+    }
     object->forward_to(cast_to_oop(_compaction_top));
     assert(object->is_forwarded(), "must be forwarded");
   } else {
@@ -165,7 +169,7 @@ void G1FullGCCompactionPoint::forward_humongous(HeapRegion* hr) {
   }
 
   // Preserve the mark for the humongous object as the region was initially not compacting.
-  _collector->marker(0)->preserved_stack()->push_if_necessary(obj, obj->mark());
+  preserved_stack()->push_if_necessary(obj, obj->mark());
 
   HeapRegion* dest_hr = _compaction_regions->at(range_begin);
   obj->forward_to(cast_to_oop(dest_hr->bottom()));

src/hotspot/share/gc/g1/g1FullGCCompactionPoint.hpp

@@ -32,11 +32,13 @@
 
 class G1FullCollector;
 class HeapRegion;
+class PreservedMarks;
 
 class G1FullGCCompactionPoint : public CHeapObj<mtGC> {
   G1FullCollector* _collector;
   HeapRegion* _current_region;
   HeapWord*   _compaction_top;
+  PreservedMarks* _preserved_stack;
   GrowableArray<HeapRegion*>* _compaction_regions;
   GrowableArrayIterator<HeapRegion*> _compaction_region_iterator;
 
@@ -47,7 +49,7 @@ class G1FullGCCompactionPoint : public CHeapObj<mtGC> {
   uint find_contiguous_before(HeapRegion* hr, uint num_regions);
 
 public:
-  G1FullGCCompactionPoint(G1FullCollector* collector);
+  G1FullGCCompactionPoint(G1FullCollector* collector, PreservedMarks* preserved_stack);
   ~G1FullGCCompactionPoint();
 
   bool has_regions();
@@ -63,6 +65,16 @@ class G1FullGCCompactionPoint : public CHeapObj<mtGC> {
   HeapRegion* current_region();
 
   GrowableArray<HeapRegion*>* regions();
+
+  PreservedMarks* preserved_stack() const {
+    assert(_preserved_stack != nullptr, "must be initialized");
+    return _preserved_stack;
+  }
+
+  void set_preserved_stack(PreservedMarks* preserved_stack) {
+    assert(_preserved_stack == nullptr, "only initialize once");
+    _preserved_stack = preserved_stack;
+  }
 };
 
 #endif // SHARE_GC_G1_G1FULLGCCOMPACTIONPOINT_HPP

src/hotspot/share/gc/g1/g1FullGCMarker.cpp

@@ -33,14 +33,12 @@
 
 G1FullGCMarker::G1FullGCMarker(G1FullCollector* collector,
                                uint worker_id,
-                               PreservedMarks* preserved_stack,
                                G1RegionMarkStats* mark_stats) :
     _collector(collector),
     _worker_id(worker_id),
     _bitmap(collector->mark_bitmap()),
     _oop_stack(),
     _objarray_stack(),
-    _preserved_stack(preserved_stack),
     _mark_closure(worker_id, this, ClassLoaderData::_claim_stw_fullgc_mark, G1CollectedHeap::heap()->ref_processor_stw()),
     _stack_closure(this),
     _cld_closure(mark_closure(), ClassLoaderData::_claim_stw_fullgc_mark),

src/hotspot/share/gc/g1/g1FullGCMarker.hpp

@@ -28,7 +28,6 @@
 #include "gc/g1/g1FullGCOopClosures.hpp"
 #include "gc/g1/g1OopClosures.hpp"
 #include "gc/g1/g1RegionMarkStatsCache.hpp"
-#include "gc/shared/preservedMarks.hpp"
 #include "gc/shared/stringdedup/stringDedup.hpp"
 #include "gc/shared/taskqueue.hpp"
 #include "memory/iterator.hpp"
@@ -59,7 +58,6 @@ class G1FullGCMarker : public CHeapObj<mtGC> {
   // Mark stack
   OopQueue           _oop_stack;
   ObjArrayTaskQueue  _objarray_stack;
-  PreservedMarks*    _preserved_stack;
 
   // Marking closures
   G1MarkAndPushClosure  _mark_closure;
@@ -89,14 +87,12 @@ class G1FullGCMarker : public CHeapObj<mtGC> {
 public:
   G1FullGCMarker(G1FullCollector* collector,
                  uint worker_id,
-                 PreservedMarks* preserved_stack,
                  G1RegionMarkStats* mark_stats);
   ~G1FullGCMarker();
 
   // Stack getters
   OopQueue*          oop_stack()       { return &_oop_stack; }
   ObjArrayTaskQueue* objarray_stack()  { return &_objarray_stack; }
-  PreservedMarks*    preserved_stack() { return _preserved_stack; }
 
   // Marking entry points
   template <class T> inline void mark_and_push(T* p);

src/hotspot/share/gc/g1/g1FullGCMarker.inline.hpp

@@ -51,13 +51,6 @@ inline bool G1FullGCMarker::mark_object(oop obj) {
     return false;
   }
 
-  // Marked by us, preserve if needed.
-  if (_collector->is_compacting(obj)) {
-    // It is not necessary to preserve marks for objects in regions we do not
-    // compact because we do not change their headers (i.e. forward them).
-    preserved_stack()->push_if_necessary(obj, obj->mark());
-  }
-
   // Check if deduplicatable string.
   if (StringDedup::is_enabled() &&
       java_lang_String::is_instance(obj) &&