Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8296130: G1: Remove G1YoungCollector::_target_pause_time_ms
Reviewed-by: iwalulya, kbarrett
  • Loading branch information
albertnetymk committed Nov 2, 2022
1 parent 2bd24c4 commit 38c1f2a
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 43 deletions.
23 changes: 8 additions & 15 deletions src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -2789,15 +2782,15 @@ 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");

if (GCLocker::check_active_before_gc()) {
return false;
}

do_collection_pause_at_safepoint_helper(target_pause_time_ms);
do_collection_pause_at_safepoint_helper();
return true;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/gc/g1/g1CollectedHeap.hpp
Expand Up @@ -756,11 +756,11 @@ class G1CollectedHeap : public CollectedHeap {
// 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);
Expand Down
18 changes: 5 additions & 13 deletions src/hotspot/share/gc/g1/g1VMOperations.cpp
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions src/hotspot/share/gc/g1/g1VMOperations.hpp
Expand Up @@ -48,7 +48,6 @@ class VM_G1CollectFull : public VM_GC_Operation {
};

class VM_G1TryInitiateConcMark : public VM_GC_Operation {
double _target_pause_time_ms;
bool _transient_failure;
bool _cycle_already_in_progress;
bool _whitebox_attached;
Expand All @@ -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();
Expand All @@ -71,13 +69,11 @@ class VM_G1TryInitiateConcMark : public VM_GC_Operation {

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; }
Expand Down
6 changes: 2 additions & 4 deletions src/hotspot/share/gc/g1/g1YoungCollector.cpp
Expand Up @@ -1026,11 +1026,9 @@ class G1PreservedMarksSet : public PreservedMarksSet {
}
};

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()
{
Expand Down Expand Up @@ -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());
Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/gc/g1/g1YoungCollector.hpp
Expand Up @@ -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;

Expand Down Expand Up @@ -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; }
Expand Down

1 comment on commit 38c1f2a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.