From 2533e8c0ceca978c69ec6df4bee4c85f64301dc9 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 17 Jan 2024 11:53:34 +0100 Subject: [PATCH] Refactoring --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 11 +++--- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 4 ++ .../share/gc/g1/g1CollectedHeap.inline.hpp | 24 +++++++----- src/hotspot/share/gc/g1/g1ConcurrentMark.cpp | 5 +++ src/hotspot/share/gc/g1/g1FullCollector.cpp | 1 + src/hotspot/share/gc/g1/g1ThreadLocalData.hpp | 37 +++++++++++-------- .../share/gc/g1/g1YoungGCPreEvacuateTasks.cpp | 15 +++++--- src/hotspot/share/gc/g1/heapRegion.inline.hpp | 5 ++- 8 files changed, 65 insertions(+), 37 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 15fd990eacd98..db51973238024 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/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 stats = G1ThreadLocalData::get_and_reset_pinned_region_cache(thread); + if (stats.second != 0) { + region_at(stats.first)->add_pinned_object_count(stats.second); } } } diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 58ac36a734f7f..d345ab58033de 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/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(); diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp index 5970f19eb26d3..58e748be4add2 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp +++ b/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); } } diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index 533817f97246c..fc12bbfc47bd6 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/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 */); diff --git a/src/hotspot/share/gc/g1/g1FullCollector.cpp b/src/hotspot/share/gc/g1/g1FullCollector.cpp index 6ae43c770cc4e..71760e0706592 100644 --- a/src/hotspot/share/gc/g1/g1FullCollector.cpp +++ b/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); diff --git a/src/hotspot/share/gc/g1/g1ThreadLocalData.hpp b/src/hotspot/share/gc/g1/g1ThreadLocalData.hpp index 9fc8a2e76b69f..7523b8b9fe467 100644 --- a/src/hotspot/share/gc/g1/g1ThreadLocalData.hpp +++ b/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 get_and_reset_pinned_region_cache(Thread* thread) { + Pair 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 diff --git a/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.cpp b/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.cpp index e4e6a675286dd..ecc0dddfe6dc6 100644 --- a/src/hotspot/share/gc/g1/g1YoungGCPreEvacuateTasks.cpp +++ b/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 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; diff --git a/src/hotspot/share/gc/g1/heapRegion.inline.hpp b/src/hotspot/share/gc/g1/heapRegion.inline.hpp index 345dc4a094803..5f604defcf0b7 100644 --- a/src/hotspot/share/gc/g1/heapRegion.inline.hpp +++ b/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