From 2e2c0b530fe0ed3544ac8977ce74848fb65a42db Mon Sep 17 00:00:00 2001 From: Stefan Johansson Date: Tue, 24 Nov 2020 13:31:48 +0100 Subject: [PATCH 1/2] 8252645: Change time measurements in G1ServiceThread to only account remembered set work --- src/hotspot/share/gc/g1/g1RemSet.cpp | 63 +++++++++++++++------ src/hotspot/share/gc/g1/g1RemSet.hpp | 7 ++- src/hotspot/share/gc/g1/g1RemSetSummary.cpp | 13 ++--- src/hotspot/share/gc/g1/g1RemSetSummary.hpp | 10 ++-- src/hotspot/share/gc/g1/g1ServiceThread.cpp | 10 +--- src/hotspot/share/gc/g1/g1ServiceThread.hpp | 3 - 6 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1RemSet.cpp b/src/hotspot/share/gc/g1/g1RemSet.cpp index ed52178d9ba41..110c346d1c828 100644 --- a/src/hotspot/share/gc/g1/g1RemSet.cpp +++ b/src/hotspot/share/gc/g1/g1RemSet.cpp @@ -482,23 +482,6 @@ class G1RemSetScanState : public CHeapObj { } }; -G1RemSet::G1RemSet(G1CollectedHeap* g1h, - G1CardTable* ct, - G1HotCardCache* hot_card_cache) : - _scan_state(new G1RemSetScanState()), - _prev_period_summary(false), - _g1h(g1h), - _ct(ct), - _g1p(_g1h->policy()), - _hot_card_cache(hot_card_cache), - _sampling_task(NULL) { -} - -G1RemSet::~G1RemSet() { - delete _scan_state; - delete _sampling_task; -} - void G1RemSet::initialize(uint max_reserved_regions) { _scan_state->initialize(max_reserved_regions); } @@ -537,6 +520,19 @@ class G1YoungRemSetSamplingClosure : public HeapRegionClosure { // Task handling young gen remembered set sampling. class G1RemSetSamplingTask : public G1ServiceTask { + // Helper to account virtual time. + class VTimer { + double _start; + public: + VTimer() : _start(os::elapsedVTime()) { } + double duration() { return os::elapsedVTime() - _start; } + }; + + double _vtime_accum; // Accumulated virtual time. + void update_vtime_accum(double duration) { + _vtime_accum += duration; + } + // Sample the current length of remembered sets for young. // // At the end of the GC G1 determines the length of the young gen based on @@ -551,6 +547,7 @@ class G1RemSetSamplingTask : public G1ServiceTask { void sample_young_list_rs_length(SuspendibleThreadSetJoiner* sts){ G1CollectedHeap* g1h = G1CollectedHeap::heap(); G1Policy* policy = g1h->policy(); + VTimer vtime; if (policy->use_adaptive_young_list_length()) { G1YoungRemSetSamplingClosure cl(sts); @@ -562,6 +559,7 @@ class G1RemSetSamplingTask : public G1ServiceTask { policy->revise_young_list_target_length_if_necessary(cl.sampled_rs_length()); } } + update_vtime_accum(vtime.duration()); } // To avoid extensive rescheduling if the task is executed a bit early. The task is @@ -596,14 +594,45 @@ class G1RemSetSamplingTask : public G1ServiceTask { sample_young_list_rs_length(&sts); schedule(G1ConcRefinementServiceIntervalMillis); } + + double vtime_accum() { + // Only report vtime if supported by the os. + if (!os::supports_vtime()) { + return 0.0; + } + return _vtime_accum; + } }; +G1RemSet::G1RemSet(G1CollectedHeap* g1h, + G1CardTable* ct, + G1HotCardCache* hot_card_cache) : + _scan_state(new G1RemSetScanState()), + _prev_period_summary(false), + _g1h(g1h), + _ct(ct), + _g1p(_g1h->policy()), + _hot_card_cache(hot_card_cache), + _sampling_task(NULL) { +} + +G1RemSet::~G1RemSet() { + delete _scan_state; + delete _sampling_task; +} + void G1RemSet::initialize_sampling_task(G1ServiceThread* thread) { assert(_sampling_task == NULL, "Sampling task already initialized"); _sampling_task = new G1RemSetSamplingTask("Remembered Set Sampling Task"); thread->register_task(_sampling_task); } +double G1RemSet::sampling_task_vtime() { + assert(_sampling_task != NULL, "Must have been initialized"); + return _sampling_task->vtime_accum(); +} + + // Helper class to scan and detect ranges of cards that need to be scanned on the // card table. class G1CardTableScanner : public StackObj { diff --git a/src/hotspot/share/gc/g1/g1RemSet.hpp b/src/hotspot/share/gc/g1/g1RemSet.hpp index ef0a7f7a37108..5bea9b76af922 100644 --- a/src/hotspot/share/gc/g1/g1RemSet.hpp +++ b/src/hotspot/share/gc/g1/g1RemSet.hpp @@ -48,8 +48,8 @@ class G1RemSetScanState; class G1ParScanThreadState; class G1ParScanThreadStateSet; class G1Policy; +class G1RemSetSamplingTask; class G1ScanCardClosure; -class G1ServiceTask; class G1ServiceThread; class HeapRegionClaimer; @@ -67,7 +67,7 @@ class G1RemSet: public CHeapObj { G1CardTable* _ct; G1Policy* _g1p; G1HotCardCache* _hot_card_cache; - G1ServiceTask* _sampling_task; + G1RemSetSamplingTask* _sampling_task; void print_merge_heap_roots_stats(); @@ -87,6 +87,9 @@ class G1RemSet: public CHeapObj { // Initialize and schedule young remembered set sampling task. void initialize_sampling_task(G1ServiceThread* thread); + // Accumulated vtime used by the sampling task. + double sampling_task_vtime(); + // Scan all cards in the non-collection set regions that potentially contain // references into the current whole collection set. void scan_heap_roots(G1ParScanThreadState* pss, diff --git a/src/hotspot/share/gc/g1/g1RemSetSummary.cpp b/src/hotspot/share/gc/g1/g1RemSetSummary.cpp index d7adde8d0baae..f1faba1f5d274 100644 --- a/src/hotspot/share/gc/g1/g1RemSetSummary.cpp +++ b/src/hotspot/share/gc/g1/g1RemSetSummary.cpp @@ -30,7 +30,6 @@ #include "gc/g1/g1DirtyCardQueue.hpp" #include "gc/g1/g1RemSet.hpp" #include "gc/g1/g1RemSetSummary.hpp" -#include "gc/g1/g1ServiceThread.hpp" #include "gc/g1/heapRegion.hpp" #include "gc/g1/heapRegionRemSet.hpp" #include "memory/allocation.inline.hpp" @@ -53,7 +52,7 @@ void G1RemSetSummary::update() { g1h->concurrent_refine()->threads_do(&collector); _num_coarsenings = HeapRegionRemSet::n_coarsenings(); - set_service_thread_vtime(g1h->service_thread()->vtime_accum()); + set_sampling_task_vtime(g1h->rem_set()->sampling_task_vtime()); } void G1RemSetSummary::set_rs_thread_vtime(uint thread, double value) { @@ -72,7 +71,7 @@ G1RemSetSummary::G1RemSetSummary(bool should_update) : _num_coarsenings(0), _num_vtimes(G1ConcurrentRefine::max_num_threads()), _rs_threads_vtimes(NEW_C_HEAP_ARRAY(double, _num_vtimes, mtGC)), - _service_thread_vtime(0.0f) { + _sampling_task_vtime(0.0f) { memset(_rs_threads_vtimes, 0, sizeof(double) * _num_vtimes); @@ -93,7 +92,7 @@ void G1RemSetSummary::set(G1RemSetSummary* other) { memcpy(_rs_threads_vtimes, other->_rs_threads_vtimes, sizeof(double) * _num_vtimes); - set_service_thread_vtime(other->service_thread_vtime()); + set_sampling_task_vtime(other->sampling_task_vtime()); } void G1RemSetSummary::subtract_from(G1RemSetSummary* other) { @@ -106,7 +105,7 @@ void G1RemSetSummary::subtract_from(G1RemSetSummary* other) { set_rs_thread_vtime(i, other->rs_thread_vtime(i) - rs_thread_vtime(i)); } - _service_thread_vtime = other->service_thread_vtime() - _service_thread_vtime; + _sampling_task_vtime = other->sampling_task_vtime() - _sampling_task_vtime; } class RegionTypeCounter { @@ -327,8 +326,8 @@ void G1RemSetSummary::print_on(outputStream* out) { out->print(" %5.2f", rs_thread_vtime(i)); } out->cr(); - out->print_cr(" Service thread time (s)"); - out->print_cr(" %5.2f", service_thread_vtime()); + out->print_cr(" Sampling task time (ms)"); + out->print_cr(" %5.3f", sampling_task_vtime() * MILLIUNITS); HRRSStatsIter blk; G1CollectedHeap::heap()->heap_region_iterate(&blk); diff --git a/src/hotspot/share/gc/g1/g1RemSetSummary.hpp b/src/hotspot/share/gc/g1/g1RemSetSummary.hpp index 1a1809210473b..4d4d326575586 100644 --- a/src/hotspot/share/gc/g1/g1RemSetSummary.hpp +++ b/src/hotspot/share/gc/g1/g1RemSetSummary.hpp @@ -38,11 +38,11 @@ class G1RemSetSummary { size_t _num_vtimes; double* _rs_threads_vtimes; - double _service_thread_vtime; + double _sampling_task_vtime; void set_rs_thread_vtime(uint thread, double value); - void set_service_thread_vtime(double value) { - _service_thread_vtime = value; + void set_sampling_task_vtime(double value) { + _sampling_task_vtime = value; } // update this summary with current data from various places @@ -62,8 +62,8 @@ class G1RemSetSummary { double rs_thread_vtime(uint thread) const; - double service_thread_vtime() const { - return _service_thread_vtime; + double sampling_task_vtime() const { + return _sampling_task_vtime; } size_t num_coarsenings() const { diff --git a/src/hotspot/share/gc/g1/g1ServiceThread.cpp b/src/hotspot/share/gc/g1/g1ServiceThread.cpp index 4c958bef23655..0d8cbb684986a 100644 --- a/src/hotspot/share/gc/g1/g1ServiceThread.cpp +++ b/src/hotspot/share/gc/g1/g1ServiceThread.cpp @@ -102,8 +102,7 @@ G1ServiceThread::G1ServiceThread() : true, Monitor::_safepoint_check_never), _task_queue(), - _periodic_gc_task(new G1PeriodicGCTask("Periodic GC Task")), - _vtime_accum(0) { + _periodic_gc_task(new G1PeriodicGCTask("Periodic GC Task")) { set_name("G1 Service"); create_and_start(); } @@ -215,8 +214,6 @@ void G1ServiceThread::run_task(G1ServiceTask* task) { } void G1ServiceThread::run_service() { - double vtime_start = os::elapsedVTime(); - // Register the tasks handled by the service thread. register_task(_periodic_gc_task); @@ -226,11 +223,6 @@ void G1ServiceThread::run_service() { run_task(task); } - if (os::supports_vtime()) { - _vtime_accum = (os::elapsedVTime() - vtime_start); - } else { - _vtime_accum = 0.0; - } sleep_before_next_cycle(); } diff --git a/src/hotspot/share/gc/g1/g1ServiceThread.hpp b/src/hotspot/share/gc/g1/g1ServiceThread.hpp index f26525400dd23..07a23773ac6ea 100644 --- a/src/hotspot/share/gc/g1/g1ServiceThread.hpp +++ b/src/hotspot/share/gc/g1/g1ServiceThread.hpp @@ -107,8 +107,6 @@ class G1ServiceThread: public ConcurrentGCThread { G1PeriodicGCTask* _periodic_gc_task; - double _vtime_accum; // Accumulated virtual time. - void run_service(); void stop_service(); @@ -133,7 +131,6 @@ class G1ServiceThread: public ConcurrentGCThread { G1ServiceThread(); ~G1ServiceThread(); - double vtime_accum() { return _vtime_accum; } // Register a task with the service thread and schedule it. If // no delay is specified the task is scheduled to run directly. void register_task(G1ServiceTask* task, jlong delay = 0); From c9dc99b8470c616e5d9422095583681582d5de07 Mon Sep 17 00:00:00 2001 From: Stefan Johansson Date: Thu, 26 Nov 2020 20:50:06 +0100 Subject: [PATCH 2/2] Thomas review --- src/hotspot/share/gc/g1/g1RemSet.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1RemSet.cpp b/src/hotspot/share/gc/g1/g1RemSet.cpp index 110c346d1c828..2c06d24f4aea1 100644 --- a/src/hotspot/share/gc/g1/g1RemSet.cpp +++ b/src/hotspot/share/gc/g1/g1RemSet.cpp @@ -482,10 +482,6 @@ class G1RemSetScanState : public CHeapObj { } }; -void G1RemSet::initialize(uint max_reserved_regions) { - _scan_state->initialize(max_reserved_regions); -} - class G1YoungRemSetSamplingClosure : public HeapRegionClosure { SuspendibleThreadSetJoiner* _sts; size_t _regions_visited; @@ -621,6 +617,10 @@ G1RemSet::~G1RemSet() { delete _sampling_task; } +void G1RemSet::initialize(uint max_reserved_regions) { + _scan_state->initialize(max_reserved_regions); +} + void G1RemSet::initialize_sampling_task(G1ServiceThread* thread) { assert(_sampling_task == NULL, "Sampling task already initialized"); _sampling_task = new G1RemSetSamplingTask("Remembered Set Sampling Task"); @@ -632,7 +632,6 @@ double G1RemSet::sampling_task_vtime() { return _sampling_task->vtime_accum(); } - // Helper class to scan and detect ranges of cards that need to be scanned on the // card table. class G1CardTableScanner : public StackObj {