From 38c1f2a70e98830f69132dc7401a1a592b3a072a Mon Sep 17 00:00:00 2001 From: Albert Mingkun Yang Date: Wed, 2 Nov 2022 13:01:12 +0000 Subject: [PATCH] 8296130: G1: Remove G1YoungCollector::_target_pause_time_ms Reviewed-by: iwalulya, kbarrett --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 23 +++++++------------- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 4 ++-- src/hotspot/share/gc/g1/g1VMOperations.cpp | 18 +++++---------- src/hotspot/share/gc/g1/g1VMOperations.hpp | 8 ++----- src/hotspot/share/gc/g1/g1YoungCollector.cpp | 6 ++--- src/hotspot/share/gc/g1/g1YoungCollector.hpp | 4 +--- 6 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 786cd134e9a..e390ae4d842 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -2023,9 +2023,7 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause, // Try to schedule concurrent start evacuation pause that will // start a concurrent cycle. LOG_COLLECT_CONCURRENTLY(cause, "attempt %u", i); - VM_G1TryInitiateConcMark op(gc_counter, - cause, - policy()->max_pause_time_ms()); + VM_G1TryInitiateConcMark op(gc_counter, cause); VMThread::execute(&op); // Request is trivially finished. @@ -2199,8 +2197,7 @@ bool G1CollectedHeap::try_collect(GCCause::Cause cause, // to 0 which means that we are not requesting a post-GC allocation. VM_G1CollectForAllocation op(0, /* word_size */ counters_before.total_collections(), - cause, - policy()->max_pause_time_ms()); + cause); VMThread::execute(&op); return op.gc_succeeded(); } else { @@ -2220,8 +2217,7 @@ void G1CollectedHeap::start_concurrent_gc_for_metadata_allocation(GCCause::Cause // will do so if one is not already in progress. bool should_start = policy()->force_concurrent_start_if_outside_cycle(gc_cause); if (should_start) { - double pause_target = policy()->max_pause_time_ms(); - do_collection_pause_at_safepoint(pause_target); + do_collection_pause_at_safepoint(); } } @@ -2650,10 +2646,7 @@ HeapWord* G1CollectedHeap::do_collection_pause(size_t word_size, bool* succeeded, GCCause::Cause gc_cause) { assert_heap_not_locked_and_not_at_safepoint(); - VM_G1CollectForAllocation op(word_size, - gc_count_before, - gc_cause, - policy()->max_pause_time_ms()); + VM_G1CollectForAllocation op(word_size, gc_count_before, gc_cause); VMThread::execute(&op); HeapWord* result = op.result(); @@ -2789,7 +2782,7 @@ void G1CollectedHeap::expand_heap_after_young_collection(){ } } -bool G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) { +bool G1CollectedHeap::do_collection_pause_at_safepoint() { assert_at_safepoint_on_vm_thread(); guarantee(!is_gc_active(), "collection is not reentrant"); @@ -2797,7 +2790,7 @@ bool G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ return false; } - do_collection_pause_at_safepoint_helper(target_pause_time_ms); + do_collection_pause_at_safepoint_helper(); return true; } @@ -2858,7 +2851,7 @@ void G1CollectedHeap::retire_tlabs() { ensure_parsability(true); } -void G1CollectedHeap::do_collection_pause_at_safepoint_helper(double target_pause_time_ms) { +void G1CollectedHeap::do_collection_pause_at_safepoint_helper() { ResourceMark rm; IsGCActiveMark active_gc_mark; @@ -2876,7 +2869,7 @@ void G1CollectedHeap::do_collection_pause_at_safepoint_helper(double target_paus bool should_start_concurrent_mark_operation = collector_state()->in_concurrent_start_gc(); // Perform the collection. - G1YoungCollector collector(gc_cause(), target_pause_time_ms); + G1YoungCollector collector(gc_cause()); collector.collect(); // It should now be safe to tell the concurrent mark thread to start diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 45f68520946..cfd9ccfe414 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -756,11 +756,11 @@ private: // active, true otherwise. // precondition: at safepoint on VM thread // precondition: !is_gc_active() - bool do_collection_pause_at_safepoint(double target_pause_time_ms); + bool do_collection_pause_at_safepoint(); // Helper for do_collection_pause_at_safepoint, containing the guts // of the incremental collection pause, executed by the vm thread. - void do_collection_pause_at_safepoint_helper(double target_pause_time_ms); + void do_collection_pause_at_safepoint_helper(); G1HeapVerifier::G1VerifyType young_collection_verify_type() const; void verify_before_young_collection(G1HeapVerifier::G1VerifyType type); diff --git a/src/hotspot/share/gc/g1/g1VMOperations.cpp b/src/hotspot/share/gc/g1/g1VMOperations.cpp index cd7f09ac2ef..f149d3f1e2b 100644 --- a/src/hotspot/share/gc/g1/g1VMOperations.cpp +++ b/src/hotspot/share/gc/g1/g1VMOperations.cpp @@ -57,10 +57,8 @@ void VM_G1CollectFull::doit() { } VM_G1TryInitiateConcMark::VM_G1TryInitiateConcMark(uint gc_count_before, - GCCause::Cause gc_cause, - double target_pause_time_ms) : + GCCause::Cause gc_cause) : VM_GC_Operation(gc_count_before, gc_cause), - _target_pause_time_ms(target_pause_time_ms), _transient_failure(false), _cycle_already_in_progress(false), _whitebox_attached(false), @@ -106,7 +104,7 @@ void VM_G1TryInitiateConcMark::doit() { // request will be remembered for a later partial collection, even though // we've rejected this request. _whitebox_attached = true; - } else if (!g1h->do_collection_pause_at_safepoint(_target_pause_time_ms)) { + } else if (!g1h->do_collection_pause_at_safepoint()) { // Failure to perform the collection at all occurs because GCLocker is // active, and we have the bad luck to be the collection request that // makes a later _gc_locker collection needed. (Else we would have hit @@ -121,15 +119,9 @@ void VM_G1TryInitiateConcMark::doit() { VM_G1CollectForAllocation::VM_G1CollectForAllocation(size_t word_size, uint gc_count_before, - GCCause::Cause gc_cause, - double target_pause_time_ms) : + GCCause::Cause gc_cause) : VM_CollectForAllocation(word_size, gc_count_before, gc_cause), - _gc_succeeded(false), - _target_pause_time_ms(target_pause_time_ms) { - - guarantee(target_pause_time_ms > 0.0, - "target_pause_time_ms = %1.6lf should be positive", - target_pause_time_ms); + _gc_succeeded(false) { _gc_cause = gc_cause; } @@ -155,7 +147,7 @@ void VM_G1CollectForAllocation::doit() { GCCauseSetter x(g1h, _gc_cause); // Try a partial collection of some kind. - _gc_succeeded = g1h->do_collection_pause_at_safepoint(_target_pause_time_ms); + _gc_succeeded = g1h->do_collection_pause_at_safepoint(); if (_gc_succeeded) { if (_word_size > 0) { diff --git a/src/hotspot/share/gc/g1/g1VMOperations.hpp b/src/hotspot/share/gc/g1/g1VMOperations.hpp index 72f450ad3bc..edf53073789 100644 --- a/src/hotspot/share/gc/g1/g1VMOperations.hpp +++ b/src/hotspot/share/gc/g1/g1VMOperations.hpp @@ -48,7 +48,6 @@ public: }; class VM_G1TryInitiateConcMark : public VM_GC_Operation { - double _target_pause_time_ms; bool _transient_failure; bool _cycle_already_in_progress; bool _whitebox_attached; @@ -57,8 +56,7 @@ class VM_G1TryInitiateConcMark : public VM_GC_Operation { public: VM_G1TryInitiateConcMark(uint gc_count_before, - GCCause::Cause gc_cause, - double target_pause_time_ms); + GCCause::Cause gc_cause); virtual VMOp_Type type() const { return VMOp_G1TryInitiateConcMark; } virtual bool doit_prologue(); virtual void doit(); @@ -71,13 +69,11 @@ public: class VM_G1CollectForAllocation : public VM_CollectForAllocation { bool _gc_succeeded; - double _target_pause_time_ms; public: VM_G1CollectForAllocation(size_t word_size, uint gc_count_before, - GCCause::Cause gc_cause, - double target_pause_time_ms); + GCCause::Cause gc_cause); virtual VMOp_Type type() const { return VMOp_G1CollectForAllocation; } virtual void doit(); bool gc_succeeded() const { return _gc_succeeded; } diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.cpp b/src/hotspot/share/gc/g1/g1YoungCollector.cpp index 205a1aa4b80..7c26b38e8a9 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.cpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.cpp @@ -1026,11 +1026,9 @@ public: } }; -G1YoungCollector::G1YoungCollector(GCCause::Cause gc_cause, - double target_pause_time_ms) : +G1YoungCollector::G1YoungCollector(GCCause::Cause gc_cause) : _g1h(G1CollectedHeap::heap()), _gc_cause(gc_cause), - _target_pause_time_ms(target_pause_time_ms), _concurrent_operation_is_full_mark(false), _evac_failure_regions() { @@ -1075,7 +1073,7 @@ void G1YoungCollector::collect() { // other trivial setup above). policy()->record_young_collection_start(); - calculate_collection_set(jtm.evacuation_info(), _target_pause_time_ms); + calculate_collection_set(jtm.evacuation_info(), policy()->max_pause_time_ms()); G1RedirtyCardsQueueSet rdcqs(G1BarrierSet::dirty_card_queue_set().allocator()); G1PreservedMarksSet preserved_marks_set(workers()->active_workers()); diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.hpp b/src/hotspot/share/gc/g1/g1YoungCollector.hpp index 63666900d0b..58f376b8b1d 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.hpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.hpp @@ -82,7 +82,6 @@ class G1YoungCollector { G1YoungGCEvacFailureInjector* evac_failure_injector() const; GCCause::Cause _gc_cause; - double _target_pause_time_ms; bool _concurrent_operation_is_full_mark; @@ -137,8 +136,7 @@ class G1YoungCollector { bool evacuation_failed() const; public: - G1YoungCollector(GCCause::Cause gc_cause, - double target_pause_time_ms); + G1YoungCollector(GCCause::Cause gc_cause); void collect(); bool concurrent_operation_is_full_mark() const { return _concurrent_operation_is_full_mark; }