Refactoring

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

@@ -2469,12 +2469,13 @@ void G1CollectedHeap::prepare_for_mutator_after_young_collection() {
 
 void G1CollectedHeap::retire_tlabs() {
   ensure_parsability(true);
+}
 
-  for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thread = jtiwh.next();) {
-    uint cached_pinned_region_idx = G1ThreadLocalData::last_pinned_region_idx(thread);
-    size_t pin_count = G1ThreadLocalData::set_new_last_pinned_region_idx(thread, 0, 0);
-    if (pin_count != 0) {
-      region_at(cached_pinned_region_idx)->add_pinned_object_count(pin_count);
+void G1CollectedHeap::flush_region_pin_cache() {
+  for (JavaThreadIteratorWithHandle jtiwh; JavaThread *thread = jtiwh.next(); ) {
+    Pair<uint, size_t> stats = G1ThreadLocalData::get_and_reset_pinned_region_cache(thread);
+    if (stats.second != 0) {
+      region_at(stats.first)->add_pinned_object_count(stats.second);
     }
   }
 }

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

@@ -775,6 +775,10 @@ class G1CollectedHeap : public CollectedHeap {
 
   void retire_tlabs();
 
+  // Update all region's pin counts from the per-thread caches. Must be called
+  // before any decision based on pin counts.
+  void flush_region_pin_cache();
+
   void expand_heap_after_young_collection();
   // Update object copying statistics.
   void record_obj_copy_mem_stats();

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

@@ -266,15 +266,18 @@ inline void G1CollectedHeap::pin_object(JavaThread* thread, oop obj) {
   assert(obj != nullptr, "obj must not be null");
   assert(!is_gc_active(), "must not pin objects during a GC");
   assert(obj->is_typeArray(), "must be typeArray");
-  HeapRegion* r = heap_region_containing(obj);
 
-  uint last_idx = G1ThreadLocalData::last_pinned_region_idx(thread);
+  uint last_idx = G1ThreadLocalData::cached_pinned_region_idx(thread);
+  HeapRegion* r = heap_region_containing(obj);
   if (last_idx == r->hrm_index()) {
-    G1ThreadLocalData::increment_pinned_region_count(thread);
+    G1ThreadLocalData::inc_cached_pinned_region_count(thread);
   } else {
     // Flush old.
-    region_at(last_idx)->add_pinned_object_count(G1ThreadLocalData::pinned_region_count(thread));
-    G1ThreadLocalData::set_new_last_pinned_region_idx(thread, r->hrm_index(), (size_t)1);
+    size_t pin_count = G1ThreadLocalData::cached_pinned_region_count(thread);
+    if (pin_count != 0) {
+      region_at(last_idx)->add_pinned_object_count(pin_count);
+    }
+    G1ThreadLocalData::get_and_set_pinned_region_cache(thread, r->hrm_index(), (size_t)1);
   }
 }
 
@@ -283,13 +286,16 @@ inline void G1CollectedHeap::unpin_object(JavaThread* thread, oop obj) {
   assert(!is_gc_active(), "must not unpin objects during a GC");
   HeapRegion* r = heap_region_containing(obj);
 
-  uint last_idx = G1ThreadLocalData::last_pinned_region_idx(thread);
+  uint last_idx = G1ThreadLocalData::cached_pinned_region_idx(thread);
   if (last_idx == r->hrm_index()) {
-    G1ThreadLocalData::decrement_pinned_region_count(thread);
+    G1ThreadLocalData::dec_cached_pinned_region_count(thread);
   } else {
     // Flush old.
-    region_at(last_idx)->add_pinned_object_count(G1ThreadLocalData::pinned_region_count(thread));
-    G1ThreadLocalData::set_new_last_pinned_region_idx(thread, r->hrm_index(), (size_t)~0);
+    size_t pin_count = G1ThreadLocalData::cached_pinned_region_count(thread);
+    if (pin_count != 0) {
+      region_at(last_idx)->add_pinned_object_count(pin_count);
+    }
+    G1ThreadLocalData::get_and_set_pinned_region_cache(thread, r->hrm_index(), (size_t)~0);
   }
 }
 

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

@@ -1347,6 +1347,11 @@ void G1ConcurrentMark::remark() {
       flush_all_task_caches();
     }
 
+    {
+      GCTraceTime(Debug, gc, phases) debug("Flush Region Pin Caches", _gc_timer_cm);
+      _g1h->flush_region_pin_cache();
+    }
+
     // All marking completed. Check bitmap now as we will start to reset TAMSes
     // in parallel below so that we can not do this in the After-Remark verification.
     _g1h->verifier()->verify_bitmap_clear(true /* above_tams_only */);

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

@@ -189,6 +189,7 @@ void G1FullCollector::prepare_collection() {
 
   _heap->gc_prologue(true);
   _heap->retire_tlabs();
+  _heap->flush_region_pin_cache();
   _heap->prepare_heap_for_full_collection();
 
   PrepareRegionsClosure cl(this);

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

@@ -37,11 +37,14 @@ class G1ThreadLocalData {
   SATBMarkQueue _satb_mark_queue;
   G1DirtyCardQueue _dirty_card_queue;
 
+  uint _cached_pinned_region_idx;
+  size_t _cached_pinned_region_count;
+
   G1ThreadLocalData() :
       _satb_mark_queue(&G1BarrierSet::satb_mark_queue_set()),
       _dirty_card_queue(&G1BarrierSet::dirty_card_queue_set()),
-      _last_pinned_region_idx(0),
-      _pinned_region_count(0) {}
+      _cached_pinned_region_idx(0),
+      _cached_pinned_region_count(0) {}
 
   static G1ThreadLocalData* data(Thread* thread) {
     assert(UseG1GC, "Sanity");
@@ -93,32 +96,34 @@ class G1ThreadLocalData {
     return dirty_card_queue_offset() + G1DirtyCardQueue::byte_offset_of_buf();
   }
 
-  static uint last_pinned_region_idx(Thread* thread) {
-    return data(thread)->_last_pinned_region_idx;
+  static uint cached_pinned_region_idx(Thread* thread) {
+    return data(thread)->_cached_pinned_region_idx;
   }
 
-  static size_t pinned_region_count(Thread* thread) {
-    return data(thread)->_pinned_region_count;
+  static size_t cached_pinned_region_count(Thread* thread) {
+    return data(thread)->_cached_pinned_region_count;
   }
 
-  static void increment_pinned_region_count(Thread* thread) {
-    data(thread)->_pinned_region_count++;
+  static void inc_cached_pinned_region_count(Thread* thread) {
+    ++data(thread)->_cached_pinned_region_count;
   }
 
-  static void decrement_pinned_region_count(Thread* thread) {
-    data(thread)->_pinned_region_count--;
+  static void dec_cached_pinned_region_count(Thread* thread) {
+    --data(thread)->_cached_pinned_region_count;
   }
 
-  static size_t set_new_last_pinned_region_idx(Thread* thread, uint region_idx, size_t new_count) {
+  static size_t get_and_set_pinned_region_cache(Thread* thread, uint region_idx, size_t new_count) {
     G1ThreadLocalData* d = data(thread);
-    size_t result = d->_pinned_region_count;
-    d->_last_pinned_region_idx = region_idx;
-    d->_pinned_region_count = new_count;
+    size_t result = d->_cached_pinned_region_count;
+    d->_cached_pinned_region_idx = region_idx;
+    d->_cached_pinned_region_count = new_count;
     return result;
   }
 
-  uint _last_pinned_region_idx;
-  size_t _pinned_region_count;
+  static Pair<uint, size_t> get_and_reset_pinned_region_cache(Thread* thread) {
+    Pair<uint, size_t> result { data(thread)->cached_pinned_region_idx(thread), get_and_set_pinned_region_cache(thread, 0, 0) };
+    return result;
+  }
 };
 
 #endif // SHARE_GC_G1_G1THREADLOCALDATA_HPP

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

@@ -28,6 +28,7 @@
 #include "gc/g1/g1ConcurrentRefineStats.hpp"
 #include "gc/g1/g1DirtyCardQueue.hpp"
 #include "gc/g1/g1YoungGCPreEvacuateTasks.hpp"
+#include "gc/g1/g1ThreadLocalData.hpp"
 #include "gc/shared/barrierSet.inline.hpp"
 #include "gc/shared/threadLocalAllocBuffer.inline.hpp"
 #include "memory/allocation.inline.hpp"
@@ -57,17 +58,17 @@ class G1PreEvacuateCollectionSetBatchTask::JavaThreadRetireTLABAndFlushLogs : pu
       assert(thread->is_Java_thread(), "must be");
       // Flushes deferred card marks, so must precede concatenating logs.
       BarrierSet::barrier_set()->make_parsable((JavaThread*)thread);
+      // Retire TLABs.
       if (UseTLAB) {
         thread->tlab().retire(&_tlab_stats);
       }
-
+      // Concatenate logs.
       G1DirtyCardQueueSet& qset = G1BarrierSet::dirty_card_queue_set();
       _refinement_stats += qset.concatenate_log_and_stats(thread);
-
-      uint cached_pinned_region_idx = G1ThreadLocalData::last_pinned_region_idx(thread);
-      size_t pin_count = G1ThreadLocalData::set_new_last_pinned_region_idx(thread, 0, 0);
-      if (pin_count != 0) {
-        G1CollectedHeap::heap()->region_at(cached_pinned_region_idx)->add_pinned_object_count(pin_count);
+      // Flush region pincount cache.
+      Pair<uint, size_t> region_pin_cache = G1ThreadLocalData::get_and_reset_pinned_region_cache(thread);
+      if (region_pin_cache.second != 0) {
+        G1CollectedHeap::heap()->region_at(region_pin_cache.first)->add_pinned_object_count(region_pin_cache.second);
       }
     }
   };
@@ -138,6 +139,8 @@ class G1PreEvacuateCollectionSetBatchTask::NonJavaThreadFlushLogs : public G1Abs
     void do_thread(Thread* thread) override {
       G1DirtyCardQueueSet& qset = G1BarrierSet::dirty_card_queue_set();
       _refinement_stats += qset.concatenate_log_and_stats(thread);
+
+      assert(G1ThreadLocalData::cached_pinned_region_count(thread) == 0, "NonJava thread pinned Java objects");
     }
   } _tc;
 

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

@@ -554,8 +554,11 @@ inline void HeapRegion::record_surv_words_in_group(size_t words_survived) {
 }
 
 inline void HeapRegion::add_pinned_object_count(size_t value) {
+  assert(!is_free(), "trying to pin free region %u, adding %zu", hrm_index(), value);
   Atomic::add(&_pinned_object_count, value, memory_order_relaxed);
-  Atomic::inc(&_total_pinned_atomicops);
+  if (UseNewCode) {
+    Atomic::inc(&_total_pinned_atomicops);
+  }
 }
 
 #endif // SHARE_GC_G1_HEAPREGION_INLINE_HPP