stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8271217: Fix race between G1PeriodicGCTask checks and GC request

Browse Source 
Reviewed-by: iwalulya, tschatzl, lkorinth
This commit is contained in:
Kim Barrett 2021-08-04 15:04:55 +00:00
parent 221e4b9c61
commit 452f7d764f
8 changed files with 146 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
View File

@ -42,6 +42,7 @@
#include "gc/g1/g1DirtyCardQueue.hpp"
#include "gc/g1/g1EvacStats.inline.hpp"
#include "gc/g1/g1FullCollector.hpp"
#include "gc/g1/g1GCCounters.hpp"
#include "gc/g1/g1GCParPhaseTimesTracker.hpp"
#include "gc/g1/g1GCPhaseTimes.hpp"
#include "gc/g1/g1GCPauseType.hpp"
@ -2015,8 +2016,14 @@ void G1CollectedHeap::increment_old_marking_cycles_completed(bool concurrent,
ml.notify_all();
}
// Helper for collect().
static G1GCCounters collection_counters(G1CollectedHeap* g1h) {
MutexLocker ml(Heap_lock);
return G1GCCounters(g1h);
}
void G1CollectedHeap::collect(GCCause::Cause cause) {
try_collect(cause);
try_collect(cause, collection_counters(this));
}
// Return true if (x < y) with allowance for wraparound.
@ -2211,25 +2218,13 @@ bool G1CollectedHeap::try_collect_concurrently(GCCause::Cause cause,
}
}
bool G1CollectedHeap::try_collect(GCCause::Cause cause) {
assert_heap_not_locked();
// Lock to get consistent set of values.
uint gc_count_before;
uint full_gc_count_before;
uint old_marking_started_before;
{
MutexLocker ml(Heap_lock);
gc_count_before = total_collections();
full_gc_count_before = total_full_collections();
old_marking_started_before = _old_marking_cycles_started;
}
bool G1CollectedHeap::try_collect(GCCause::Cause cause,
const G1GCCounters& counters_before) {
if (should_do_concurrent_full_gc(cause)) {
return try_collect_concurrently(cause,
gc_count_before,
old_marking_started_before);
} else if (GCLocker::should_discard(cause, gc_count_before)) {
counters_before.total_collections(),
counters_before.old_marking_cycles_started());
} else if (GCLocker::should_discard(cause, counters_before.total_collections())) {
// Indicate failure to be consistent with VMOp failure due to
// another collection slipping in after our gc_count but before
// our request is processed.
@ -2240,14 +2235,16 @@ bool G1CollectedHeap::try_collect(GCCause::Cause cause) {
// Schedule a standard evacuation pause. We're setting word_size
// to 0 which means that we are not requesting a post-GC allocation.
VM_G1CollectForAllocation op(0, /* word_size */
gc_count_before,
counters_before.total_collections(),
cause,
policy()->max_pause_time_ms());
VMThread::execute(&op);
return op.gc_succeeded();
} else {
// Schedule a Full GC.
VM_G1CollectFull op(gc_count_before, full_gc_count_before, cause);
VM_G1CollectFull op(counters_before.total_collections(),
counters_before.total_full_collections(),
cause);
VMThread::execute(&op);
return op.gc_succeeded();
}

9
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
View File

@ -81,6 +81,7 @@ class Space;
class G1BatchedGangTask;
class G1CardTableEntryClosure;
class G1CollectionSet;
class G1GCCounters;
class G1Policy;
class G1HotCardCache;
class G1RemSet;
@ -670,7 +671,11 @@ public:
// to only parts, or aborted before completion).
void increment_old_marking_cycles_completed(bool concurrent, bool whole_heap_examined);
uint old_marking_cycles_completed() {
uint old_marking_cycles_started() const {
return _old_marking_cycles_started;
}
uint old_marking_cycles_completed() const {
return _old_marking_cycles_completed;
}
@ -1136,7 +1141,7 @@ public:
// Perform a collection of the heap with the given cause.
// Returns whether this collection actually executed.
bool try_collect(GCCause::Cause cause);
bool try_collect(GCCause::Cause cause, const G1GCCounters& counters_before);
// True iff an evacuation has failed in the most-recent collection.
inline bool evacuation_failed() const;

34
src/hotspot/share/gc/g1/g1GCCounters.cpp Normal file
View File

@ -0,0 +1,34 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/
#include "precompiled.hpp"
#include "gc/g1/g1CollectedHeap.hpp"
#include "gc/g1/g1GCCounters.hpp"
G1GCCounters::G1GCCounters(G1CollectedHeap* g1h) :
_total_collections(g1h->total_collections()),
_total_full_collections(g1h->total_full_collections()),
_old_marking_cycles_started(g1h->old_marking_cycles_started())
{}

51
src/hotspot/share/gc/g1/g1GCCounters.hpp Normal file
View File

@ -0,0 +1,51 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/
#ifndef SHARE_GC_G1_G1GCCOUNTERS_HPP
#define SHARE_GC_G1_G1GCCOUNTERS_HPP
#include "utilities/globalDefinitions.hpp"
class G1CollectedHeap;
// Record collection counters for later use when deciding whether a GC has
// been run since the counter state was recorded.
class G1GCCounters {
uint _total_collections;
uint _total_full_collections;
uint _old_marking_cycles_started;
public:
G1GCCounters() {} // Uninitialized
// Capture the current counters from the heap. The caller must ensure no
// collections will occur while this constructor is executing.
explicit G1GCCounters(G1CollectedHeap* g1h);
uint total_collections() const { return _total_collections; }
uint total_full_collections() const { return _total_full_collections; }
uint old_marking_cycles_started() const { return _old_marking_cycles_started; }
};
#endif // SHARE_GC_G1_G1GCCOUNTERS_HPP

18
src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -26,6 +26,7 @@
#include "gc/g1/g1CollectedHeap.inline.hpp"
#include "gc/g1/g1ConcurrentMark.inline.hpp"
#include "gc/g1/g1ConcurrentMarkThread.inline.hpp"
#include "gc/g1/g1GCCounters.hpp"
#include "gc/g1/g1PeriodicGCTask.hpp"
#include "gc/shared/suspendibleThreadSet.hpp"
#include "logging/log.hpp"
@ -33,11 +34,11 @@
#include "runtime/os.hpp"
#include "utilities/globalDefinitions.hpp"
bool G1PeriodicGCTask::should_start_periodic_gc() {
bool G1PeriodicGCTask::should_start_periodic_gc(G1CollectedHeap* g1h,
G1GCCounters* counters) {
// Ensure no GC safepoints while we're doing the checks, to avoid data races.
SuspendibleThreadSetJoiner sts;
G1CollectedHeap* g1h = G1CollectedHeap::heap();
// If we are currently in a concurrent mark we are going to uncommit memory soon.
if (g1h->concurrent_mark()->cm_thread()->in_progress()) {
log_debug(gc, periodic)("Concurrent cycle in progress. Skipping.");
@ -60,6 +61,11 @@ bool G1PeriodicGCTask::should_start_periodic_gc() {
recent_load, G1PeriodicGCSystemLoadThreshold);
return false;
}
// Record counters with GC safepoints blocked, to get a consistent snapshot.
// These are passed to try_collect so a GC between our release of the
// STS-joiner and the GC VMOp can be detected and cancel the request.
*counters = G1GCCounters(g1h);
return true;
}
@ -70,8 +76,10 @@ void G1PeriodicGCTask::check_for_periodic_gc() {
}
log_debug(gc, periodic)("Checking for periodic GC.");
if (should_start_periodic_gc()) {
if (!G1CollectedHeap::heap()->try_collect(GCCause::_g1_periodic_collection)) {
G1CollectedHeap* g1h = G1CollectedHeap::heap();
G1GCCounters counters;
if (should_start_periodic_gc(g1h, &counters)) {
if (!g1h->try_collect(GCCause::_g1_periodic_collection, counters)) {
log_debug(gc, periodic)("GC request denied. Skipping.");
}
}

10
src/hotspot/share/gc/g1/g1PeriodicGCTask.hpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -27,9 +27,13 @@
#include "gc/g1/g1ServiceThread.hpp"
class G1CollectedHeap;
class G1GCCounters;
// Task handling periodic GCs
class G1PeriodicGCTask : public G1ServiceTask {
bool should_start_periodic_gc();
bool should_start_periodic_gc(G1CollectedHeap* g1h,
G1GCCounters* counters);
void check_for_periodic_gc();
public:
@ -37,4 +41,4 @@ public:
virtual void execute();
};
#endif // SHARE_GC_G1_G1PERIODICGCTASK_HPP
#endif // SHARE_GC_G1_G1PERIODICGCTASK_HPP

11
src/hotspot/share/gc/g1/g1VMOperations.cpp
View File

@ -36,6 +36,17 @@
#include "memory/universe.hpp"
#include "runtime/interfaceSupport.inline.hpp"
bool VM_G1CollectFull::skip_operation() const {
// There is a race between the periodic collection task's checks for
// wanting a collection and processing its request. A collection in that
// gap should cancel the request.
if ((_gc_cause == GCCause::_g1_periodic_collection) &&
(G1CollectedHeap::heap()->total_collections() != _gc_count_before)) {
return true;
}
return VM_GC_Operation::skip_operation();
}
void VM_G1CollectFull::doit() {
G1CollectedHeap* g1h = G1CollectedHeap::heap();
GCCauseSetter x(g1h, _gc_cause);

9
src/hotspot/share/gc/g1/g1VMOperations.hpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -33,14 +33,17 @@
class VM_G1CollectFull : public VM_GC_Operation {
bool _gc_succeeded;
protected:
bool skip_operation() const override;
public:
VM_G1CollectFull(uint gc_count_before,
uint full_gc_count_before,
GCCause::Cause cause) :
VM_GC_Operation(gc_count_before, cause, full_gc_count_before, true),
_gc_succeeded(false) { }
virtual VMOp_Type type() const { return VMOp_G1CollectFull; }
virtual void doit();
VMOp_Type type() const override { return VMOp_G1CollectFull; }
void doit() override;
bool gc_succeeded() const { return _gc_succeeded; }
};