8265461: G1: Forwarding pointer removal thread sizing

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

@@ -1444,7 +1444,7 @@ G1CollectedHeap::G1CollectedHeap() :
   _cm_thread(NULL),
   _cr(NULL),
   _task_queues(NULL),
-  _evacuation_failed(false),
+  _num_regions_failed_evacuation(0),
   _evacuation_failed_info_array(NULL),
   _preserved_marks_set(true /* in_c_heap */),
 #ifndef PRODUCT
@@ -3087,8 +3087,16 @@ void G1CollectedHeap::do_collection_pause_at_safepoint_helper(double target_paus
 }
 
 void G1CollectedHeap::remove_self_forwarding_pointers(G1RedirtyCardsQueueSet* rdcqs) {
-  G1ParRemoveSelfForwardPtrsTask rsfp_task(rdcqs);
-  workers()->run_task(&rsfp_task);
+  uint num_workers = MIN2(workers()->active_workers(), num_regions_failed_evacuation());
+
+  G1ParRemoveSelfForwardPtrsTask cl(rdcqs);
+  log_debug(gc, ergo)("Running %s using %u workers for %u failed regions",
+                      cl.name(), num_workers, num_regions_failed_evacuation());
+  workers()->run_task(&cl, num_workers);
+
+  assert(cl.num_failed_regions() == num_regions_failed_evacuation(),
+         "Removed regions %u inconsistent with expected %u",
+         cl.num_failed_regions(), num_regions_failed_evacuation());
 }
 
 void G1CollectedHeap::restore_after_evac_failure(G1RedirtyCardsQueueSet* rdcqs) {
@@ -3101,10 +3109,6 @@ void G1CollectedHeap::restore_after_evac_failure(G1RedirtyCardsQueueSet* rdcqs)
 }
 
 void G1CollectedHeap::preserve_mark_during_evac_failure(uint worker_id, oop obj, markWord m) {
-  if (!_evacuation_failed) {
-    _evacuation_failed = true;
-  }
-
   _evacuation_failed_info_array[worker_id].register_copy_failure(obj->size());
   _preserved_marks_set.get(worker_id)->push_if_necessary(obj, m);
 }
@@ -3658,7 +3662,7 @@ void G1CollectedHeap::pre_evacuate_collection_set(G1EvacuationInfo& evacuation_i
   _bytes_used_during_gc = 0;
 
   _expand_heap_after_alloc_failure = true;
-  _evacuation_failed = false;
+  Atomic::store(&_num_regions_failed_evacuation, 0u);
 
   // Disable the hot card cache.
   _hot_card_cache->reset_hot_cache_claimed_index();

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

@@ -851,8 +851,8 @@ class G1CollectedHeap : public CollectedHeap {
   // The parallel task queues
   G1ScannerTasksQueueSet *_task_queues;
 
-  // True iff a evacuation has failed in the current collection.
-  bool _evacuation_failed;
+  // Number of regions evacuation failed in the current collection.
+  volatile uint _num_regions_failed_evacuation;
 
   EvacuationFailedInfo* _evacuation_failed_info_array;
 
@@ -1137,7 +1137,11 @@ class G1CollectedHeap : public CollectedHeap {
   bool try_collect(GCCause::Cause cause);
 
   // True iff an evacuation has failed in the most-recent collection.
-  bool evacuation_failed() { return _evacuation_failed; }
+  inline bool evacuation_failed() const;
+  inline uint num_regions_failed_evacuation() const;
+  // Notify that the garbage collection encountered an evacuation failure in a
+  // region. Should only be called once per region.
+  inline void notify_region_failed_evacuation();
 
   void remove_from_old_gen_sets(const uint old_regions_removed,
                                 const uint archive_regions_removed,

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

@@ -35,6 +35,7 @@
 #include "gc/g1/heapRegionSet.inline.hpp"
 #include "gc/shared/markBitMap.inline.hpp"
 #include "gc/shared/taskqueue.inline.hpp"
+#include "runtime/atomic.hpp"
 
 G1GCPhaseTimes* G1CollectedHeap::phase_times() const {
   return _policy->phase_times();
@@ -188,6 +189,18 @@ void G1CollectedHeap::register_optional_region_with_region_attr(HeapRegion* r) {
   _region_attr.set_optional(r->hrm_index(), r->rem_set()->is_tracked());
 }
 
+bool G1CollectedHeap::evacuation_failed() const {
+  return num_regions_failed_evacuation() > 0;
+}
+
+uint G1CollectedHeap::num_regions_failed_evacuation() const {
+  return Atomic::load(&_num_regions_failed_evacuation);
+}
+
+void G1CollectedHeap::notify_region_failed_evacuation() {
+  Atomic::inc(&_num_regions_failed_evacuation, memory_order_relaxed);
+}
+
 #ifndef PRODUCT
 // Support for G1EvacuationFailureALot
 

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

@@ -205,12 +205,15 @@ class RemoveSelfForwardPtrHRClosure: public HeapRegionClosure {
   G1RedirtyCardsLocalQueueSet _rdc_local_qset;
   UpdateLogBuffersDeferred _log_buffer_cl;
 
+  uint volatile* _num_failed_regions;
+
 public:
-  RemoveSelfForwardPtrHRClosure(G1RedirtyCardsQueueSet* rdcqs, uint worker_id) :
+  RemoveSelfForwardPtrHRClosure(G1RedirtyCardsQueueSet* rdcqs, uint worker_id, uint volatile* num_failed_regions) :
     _g1h(G1CollectedHeap::heap()),
     _worker_id(worker_id),
     _rdc_local_qset(rdcqs),
-    _log_buffer_cl(&_rdc_local_qset) {
+    _log_buffer_cl(&_rdc_local_qset),
+    _num_failed_regions(num_failed_regions) {
   }
 
   ~RemoveSelfForwardPtrHRClosure() {
@@ -252,6 +255,8 @@ class RemoveSelfForwardPtrHRClosure: public HeapRegionClosure {
       hr->rem_set()->clear_locked(true);
 
       hr->note_self_forwarding_removal_end(live_bytes);
+
+      Atomic::inc(_num_failed_regions, memory_order_relaxed);
     }
     return false;
   }
@@ -261,10 +266,11 @@ G1ParRemoveSelfForwardPtrsTask::G1ParRemoveSelfForwardPtrsTask(G1RedirtyCardsQue
   AbstractGangTask("G1 Remove Self-forwarding Pointers"),
   _g1h(G1CollectedHeap::heap()),
   _rdcqs(rdcqs),
-  _hrclaimer(_g1h->workers()->active_workers()) { }
+  _hrclaimer(_g1h->workers()->active_workers()),
+  _num_failed_regions(0) { }
 
 void G1ParRemoveSelfForwardPtrsTask::work(uint worker_id) {
-  RemoveSelfForwardPtrHRClosure rsfp_cl(_rdcqs, worker_id);
+  RemoveSelfForwardPtrHRClosure rsfp_cl(_rdcqs, worker_id, &_num_failed_regions);
 
   // We need to check all collection set regions whether they need self forward
   // removals, not only the last collection set increment. The reason is that
@@ -273,3 +279,7 @@ void G1ParRemoveSelfForwardPtrsTask::work(uint worker_id) {
   // might cause an evacuation failure in any region in the collection set.
   _g1h->collection_set_par_iterate_all(&rsfp_cl, &_hrclaimer, worker_id);
 }
+
+uint G1ParRemoveSelfForwardPtrsTask::num_failed_regions() const {
+  return Atomic::load(&_num_failed_regions);
+}

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

@@ -41,10 +41,14 @@ class G1ParRemoveSelfForwardPtrsTask: public AbstractGangTask {
   G1RedirtyCardsQueueSet* _rdcqs;
   HeapRegionClaimer _hrclaimer;
 
+  uint volatile _num_failed_regions;
+
 public:
   G1ParRemoveSelfForwardPtrsTask(G1RedirtyCardsQueueSet* rdcqs);
 
   void work(uint worker_id);
+
+  uint num_failed_regions() const;
 };
 
 #endif // SHARE_GC_G1_G1EVACFAILURE_HPP

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

@@ -617,9 +617,9 @@ oop G1ParScanThreadState::handle_evacuation_failure_par(oop old, markWord m) {
     // Forward-to-self succeeded. We are the "owner" of the object.
     HeapRegion* r = _g1h->heap_region_containing(old);
 
-    if (!r->evacuation_failed()) {
-      r->set_evacuation_failed(true);
-     _g1h->hr_printer()->evac_failure(r);
+    if (r->set_evacuation_failed()) {
+      _g1h->notify_region_failed_evacuation();
+      _g1h->hr_printer()->evac_failure(r);
     }
 
     _g1h->preserve_mark_during_evac_failure(_worker_id, old, m);

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

@@ -109,8 +109,9 @@ void HeapRegion::setup_heap_region_size(size_t max_heap_size) {
 void HeapRegion::handle_evacuation_failure() {
   uninstall_surv_rate_group();
   clear_young_index_in_cset();
-  set_evacuation_failed(false);
+  reset_evacuation_failed();
   set_old();
+  _next_marked_bytes = 0;
 }
 
 void HeapRegion::unlink_from_list() {
@@ -138,7 +139,7 @@ void HeapRegion::hr_clear(bool clear_space) {
   init_top_at_mark_start();
   if (clear_space) clear(SpaceDecorator::Mangle);
 
-  _evacuation_failed = false;
+  Atomic::store(&_evacuation_failed, false);
   _gc_efficiency = -1.0;
 }
 

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

@@ -207,7 +207,7 @@ class HeapRegion : public CHeapObj<mtGC> {
   HeapRegion* _humongous_start_region;
 
   // True iff an attempt to evacuate an object in the region failed.
-  bool _evacuation_failed;
+  volatile bool _evacuation_failed;
 
   static const uint InvalidCSetIndex = UINT_MAX;
 
@@ -497,16 +497,13 @@ class HeapRegion : public CHeapObj<mtGC> {
   void clear_cardtable();
 
   // Returns the "evacuation_failed" property of the region.
-  bool evacuation_failed() { return _evacuation_failed; }
+  inline bool evacuation_failed() const;
 
-  // Sets the "evacuation_failed" property of the region.
-  void set_evacuation_failed(bool b) {
-    _evacuation_failed = b;
+  // Sets the "evacuation_failed" property of the region, returning true if this
+  // has been the first call, false otherwise.
+  inline bool set_evacuation_failed();
 
-    if (b) {
-      _next_marked_bytes = 0;
-    }
-  }
+  inline void reset_evacuation_failed();
 
   // Notify the region that we are about to start processing
   // self-forwarded objects during evac failure handling.

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

@@ -451,4 +451,16 @@ inline void HeapRegion::record_surv_words_in_group(size_t words_survived) {
   _surv_rate_group->record_surviving_words(age_in_group, words_survived);
 }
 
+inline bool HeapRegion::evacuation_failed() const {
+  return Atomic::load(&_evacuation_failed);
+}
+
+inline bool HeapRegion::set_evacuation_failed() {
+  return !Atomic::load(&_evacuation_failed) && !Atomic::cmpxchg(&_evacuation_failed, false, true, memory_order_relaxed);
+}
+
+inline void HeapRegion::reset_evacuation_failed() {
+  Atomic::store(&_evacuation_failed, false);
+}
+
 #endif // SHARE_GC_G1_HEAPREGION_INLINE_HPP