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

7172388: G1: _total_full_collections should not be incremented for concurrent cycles

Browse Source 
Reviewed-by: azeemj, jmasa
This commit is contained in:
Bengt Rutisson 2012-06-05 22:30:24 +02:00
parent 1ae0edb1f1
commit e294a9f5ec
5 changed files with 57 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
hotspot/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
View File

@ -293,7 +293,7 @@ void ConcurrentMarkThread::run() {
// Java thread is waiting for a full GC to happen (e.g., it // Java thread is waiting for a full GC to happen (e.g., it
// called System.gc() with +ExplicitGCInvokesConcurrent). // called System.gc() with +ExplicitGCInvokesConcurrent).
_sts.join(); _sts.join();
g1h->increment_full_collections_completed(true /* concurrent */); g1h->increment_old_marking_cycles_completed(true /* concurrent */);
_sts.leave(); _sts.leave();
} }
assert(_should_terminate, "just checking"); assert(_should_terminate, "just checking");

52
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
View File

@ -1299,6 +1299,7 @@ bool G1CollectedHeap::do_collection(bool explicit_gc,
gc_prologue(true); gc_prologue(true);
increment_total_collections(true /* full gc */); increment_total_collections(true /* full gc */);
increment_old_marking_cycles_started();
size_t g1h_prev_used = used(); size_t g1h_prev_used = used();
assert(used() == recalculate_used(), "Should be equal"); assert(used() == recalculate_used(), "Should be equal");
@ -1501,7 +1502,7 @@ bool G1CollectedHeap::do_collection(bool explicit_gc,
"young list should be empty at this point"); "young list should be empty at this point");
// Update the number of full collections that have been completed. // Update the number of full collections that have been completed.
increment_full_collections_completed(false /* concurrent */); increment_old_marking_cycles_completed(false /* concurrent */);
_hrs.verify_optional(); _hrs.verify_optional();
verify_region_sets_optional(); verify_region_sets_optional();
@ -1888,7 +1889,8 @@ G1CollectedHeap::G1CollectedHeap(G1CollectorPolicy* policy_) :
_retained_old_gc_alloc_region(NULL), _retained_old_gc_alloc_region(NULL),
_expand_heap_after_alloc_failure(true), _expand_heap_after_alloc_failure(true),
_surviving_young_words(NULL), _surviving_young_words(NULL),
_full_collections_completed(0), _old_marking_cycles_started(0),
_old_marking_cycles_completed(0),
_in_cset_fast_test(NULL), _in_cset_fast_test(NULL),
_in_cset_fast_test_base(NULL), _in_cset_fast_test_base(NULL),
_dirty_cards_region_list(NULL), _dirty_cards_region_list(NULL),
@ -2360,7 +2362,16 @@ void G1CollectedHeap::allocate_dummy_regions() {
} }
#endif // !PRODUCT #endif // !PRODUCT
void G1CollectedHeap::increment_full_collections_completed(bool concurrent) { void G1CollectedHeap::increment_old_marking_cycles_started() {
assert(_old_marking_cycles_started == _old_marking_cycles_completed ||
_old_marking_cycles_started == _old_marking_cycles_completed + 1,
err_msg("Wrong marking cycle count (started: %d, completed: %d)",
_old_marking_cycles_started, _old_marking_cycles_completed));
_old_marking_cycles_started++;
}
void G1CollectedHeap::increment_old_marking_cycles_completed(bool concurrent) {
MonitorLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag); MonitorLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag);
// We assume that if concurrent == true, then the caller is a // We assume that if concurrent == true, then the caller is a
@ -2368,11 +2379,6 @@ void G1CollectedHeap::increment_full_collections_completed(bool concurrent) {
// Set. If there's ever a cheap way to check this, we should add an // Set. If there's ever a cheap way to check this, we should add an
// assert here. // assert here.
// We have already incremented _total_full_collections at the start
// of the GC, so total_full_collections() represents how many full
// collections have been started.
unsigned int full_collections_started = total_full_collections();
// Given that this method is called at the end of a Full GC or of a // Given that this method is called at the end of a Full GC or of a
// concurrent cycle, and those can be nested (i.e., a Full GC can // concurrent cycle, and those can be nested (i.e., a Full GC can
// interrupt a concurrent cycle), the number of full collections // interrupt a concurrent cycle), the number of full collections
@ -2382,21 +2388,21 @@ void G1CollectedHeap::increment_full_collections_completed(bool concurrent) {
// This is the case for the inner caller, i.e. a Full GC. // This is the case for the inner caller, i.e. a Full GC.
assert(concurrent || assert(concurrent ||
(full_collections_started == _full_collections_completed + 1) || (_old_marking_cycles_started == _old_marking_cycles_completed + 1) ||
(full_collections_started == _full_collections_completed + 2), (_old_marking_cycles_started == _old_marking_cycles_completed + 2),
err_msg("for inner caller (Full GC): full_collections_started = %u " err_msg("for inner caller (Full GC): _old_marking_cycles_started = %u "
"is inconsistent with _full_collections_completed = %u", "is inconsistent with _old_marking_cycles_completed = %u",
full_collections_started, _full_collections_completed)); _old_marking_cycles_started, _old_marking_cycles_completed));
// This is the case for the outer caller, i.e. the concurrent cycle. // This is the case for the outer caller, i.e. the concurrent cycle.
assert(!concurrent || assert(!concurrent ||
(full_collections_started == _full_collections_completed + 1), (_old_marking_cycles_started == _old_marking_cycles_completed + 1),
err_msg("for outer caller (concurrent cycle): " err_msg("for outer caller (concurrent cycle): "
"full_collections_started = %u " "_old_marking_cycles_started = %u "
"is inconsistent with _full_collections_completed = %u", "is inconsistent with _old_marking_cycles_completed = %u",
full_collections_started, _full_collections_completed)); _old_marking_cycles_started, _old_marking_cycles_completed));
_full_collections_completed += 1; _old_marking_cycles_completed += 1;
// We need to clear the "in_progress" flag in the CM thread before // We need to clear the "in_progress" flag in the CM thread before
// we wake up any waiters (especially when ExplicitInvokesConcurrent // we wake up any waiters (especially when ExplicitInvokesConcurrent
@ -2432,7 +2438,7 @@ void G1CollectedHeap::collect(GCCause::Cause cause) {
assert_heap_not_locked(); assert_heap_not_locked();
unsigned int gc_count_before; unsigned int gc_count_before;
unsigned int full_gc_count_before; unsigned int old_marking_count_before;
bool retry_gc; bool retry_gc;
do { do {
@ -2443,7 +2449,7 @@ void G1CollectedHeap::collect(GCCause::Cause cause) {
// Read the GC count while holding the Heap_lock // Read the GC count while holding the Heap_lock
gc_count_before = total_collections(); gc_count_before = total_collections();
full_gc_count_before = total_full_collections(); old_marking_count_before = _old_marking_cycles_started;
} }
if (should_do_concurrent_full_gc(cause)) { if (should_do_concurrent_full_gc(cause)) {
@ -2458,7 +2464,7 @@ void G1CollectedHeap::collect(GCCause::Cause cause) {
VMThread::execute(&op); VMThread::execute(&op);
if (!op.pause_succeeded()) { if (!op.pause_succeeded()) {
if (full_gc_count_before == total_full_collections()) { if (old_marking_count_before == _old_marking_cycles_started) {
retry_gc = op.should_retry_gc(); retry_gc = op.should_retry_gc();
} else { } else {
// A Full GC happened while we were trying to schedule the // A Full GC happened while we were trying to schedule the
@ -2486,7 +2492,7 @@ void G1CollectedHeap::collect(GCCause::Cause cause) {
VMThread::execute(&op); VMThread::execute(&op);
} else { } else {
// Schedule a Full GC. // Schedule a Full GC.
VM_G1CollectFull op(gc_count_before, full_gc_count_before, cause); VM_G1CollectFull op(gc_count_before, old_marking_count_before, cause);
VMThread::execute(&op); VMThread::execute(&op);
} }
} }
@ -3613,7 +3619,7 @@ G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) {
if (g1_policy()->during_initial_mark_pause()) { if (g1_policy()->during_initial_mark_pause()) {
// We are about to start a marking cycle, so we increment the // We are about to start a marking cycle, so we increment the
// full collection counter. // full collection counter.
increment_total_full_collections(); increment_old_marking_cycles_started();
} }
// if the log level is "finer" is on, we'll print long statistics information // if the log level is "finer" is on, we'll print long statistics information
// in the collector policy code, so let's not print this as the output // in the collector policy code, so let's not print this as the output

25
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp
View File

@ -359,10 +359,13 @@ private:
// (c) cause == _g1_humongous_allocation // (c) cause == _g1_humongous_allocation
bool should_do_concurrent_full_gc(GCCause::Cause cause); bool should_do_concurrent_full_gc(GCCause::Cause cause);
// Keeps track of how many "full collections" (i.e., Full GCs or // Keeps track of how many "old marking cycles" (i.e., Full GCs or
// concurrent cycles) we have completed. The number of them we have // concurrent cycles) we have started.
// started is maintained in _total_full_collections in CollectedHeap. volatile unsigned int _old_marking_cycles_started;
volatile unsigned int _full_collections_completed;
// Keeps track of how many "old marking cycles" (i.e., Full GCs or
// concurrent cycles) we have completed.
volatile unsigned int _old_marking_cycles_completed;
// This is a non-product method that is helpful for testing. It is // This is a non-product method that is helpful for testing. It is
// called at the end of a GC and artificially expands the heap by // called at the end of a GC and artificially expands the heap by
@ -673,8 +676,12 @@ public:
(size_t) _in_cset_fast_test_length * sizeof(bool)); (size_t) _in_cset_fast_test_length * sizeof(bool));
} }
// This is called at the start of either a concurrent cycle or a Full
// GC to update the number of old marking cycles started.
void increment_old_marking_cycles_started();
// This is called at the end of either a concurrent cycle or a Full // This is called at the end of either a concurrent cycle or a Full
// GC to update the number of full collections completed. Those two // GC to update the number of old marking cycles completed. Those two
// can happen in a nested fashion, i.e., we start a concurrent // can happen in a nested fashion, i.e., we start a concurrent
// cycle, a Full GC happens half-way through it which ends first, // cycle, a Full GC happens half-way through it which ends first,
// and then the cycle notices that a Full GC happened and ends // and then the cycle notices that a Full GC happened and ends
@ -683,14 +690,14 @@ public:
// false, the caller is the inner caller in the nesting (i.e., the // false, the caller is the inner caller in the nesting (i.e., the
// Full GC). If concurrent is true, the caller is the outer caller // Full GC). If concurrent is true, the caller is the outer caller
// in this nesting (i.e., the concurrent cycle). Further nesting is // in this nesting (i.e., the concurrent cycle). Further nesting is
// not currently supported. The end of the this call also notifies // not currently supported. The end of this call also notifies
// the FullGCCount_lock in case a Java thread is waiting for a full // the FullGCCount_lock in case a Java thread is waiting for a full
// GC to happen (e.g., it called System.gc() with // GC to happen (e.g., it called System.gc() with
// +ExplicitGCInvokesConcurrent). // +ExplicitGCInvokesConcurrent).
void increment_full_collections_completed(bool concurrent); void increment_old_marking_cycles_completed(bool concurrent);
unsigned int full_collections_completed() { unsigned int old_marking_cycles_completed() {
return _full_collections_completed; return _old_marking_cycles_completed;
} }
G1HRPrinter* hr_printer() { return &_hr_printer; } G1HRPrinter* hr_printer() { return &_hr_printer; }

20
hotspot/src/share/vm/gc_implementation/g1/vm_operations_g1.cpp
View File

@ -64,7 +64,7 @@ VM_G1IncCollectionPause::VM_G1IncCollectionPause(
_should_initiate_conc_mark(should_initiate_conc_mark), _should_initiate_conc_mark(should_initiate_conc_mark),
_target_pause_time_ms(target_pause_time_ms), _target_pause_time_ms(target_pause_time_ms),
_should_retry_gc(false), _should_retry_gc(false),
_full_collections_completed_before(0) { _old_marking_cycles_completed_before(0) {
guarantee(target_pause_time_ms > 0.0, guarantee(target_pause_time_ms > 0.0,
err_msg("target_pause_time_ms = %1.6lf should be positive", err_msg("target_pause_time_ms = %1.6lf should be positive",
target_pause_time_ms)); target_pause_time_ms));
@ -112,11 +112,11 @@ void VM_G1IncCollectionPause::doit() {
GCCauseSetter x(g1h, _gc_cause); GCCauseSetter x(g1h, _gc_cause);
if (_should_initiate_conc_mark) { if (_should_initiate_conc_mark) {
// It's safer to read full_collections_completed() here, given // It's safer to read old_marking_cycles_completed() here, given
// that noone else will be updating it concurrently. Since we'll // that noone else will be updating it concurrently. Since we'll
// only need it if we're initiating a marking cycle, no point in // only need it if we're initiating a marking cycle, no point in
// setting it earlier. // setting it earlier.
_full_collections_completed_before = g1h->full_collections_completed(); _old_marking_cycles_completed_before = g1h->old_marking_cycles_completed();
// At this point we are supposed to start a concurrent cycle. We // At this point we are supposed to start a concurrent cycle. We
// will do so if one is not already in progress. // will do so if one is not already in progress.
@ -181,17 +181,17 @@ void VM_G1IncCollectionPause::doit_epilogue() {
G1CollectedHeap* g1h = G1CollectedHeap::heap(); G1CollectedHeap* g1h = G1CollectedHeap::heap();
// In the doit() method we saved g1h->full_collections_completed() // In the doit() method we saved g1h->old_marking_cycles_completed()
// in the _full_collections_completed_before field. We have to // in the _old_marking_cycles_completed_before field. We have to
// wait until we observe that g1h->full_collections_completed() // wait until we observe that g1h->old_marking_cycles_completed()
// has increased by at least one. This can happen if a) we started // has increased by at least one. This can happen if a) we started
// a cycle and it completes, b) a cycle already in progress // a cycle and it completes, b) a cycle already in progress
// completes, or c) a Full GC happens. // completes, or c) a Full GC happens.
// If the condition has already been reached, there's no point in // If the condition has already been reached, there's no point in
// actually taking the lock and doing the wait. // actually taking the lock and doing the wait.
if (g1h->full_collections_completed() <= if (g1h->old_marking_cycles_completed() <=
_full_collections_completed_before) { _old_marking_cycles_completed_before) {
// The following is largely copied from CMS // The following is largely copied from CMS
Thread* thr = Thread::current(); Thread* thr = Thread::current();
@ -200,8 +200,8 @@ void VM_G1IncCollectionPause::doit_epilogue() {
ThreadToNativeFromVM native(jt); ThreadToNativeFromVM native(jt);
MutexLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag); MutexLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag);
while (g1h->full_collections_completed() <= while (g1h->old_marking_cycles_completed() <=
_full_collections_completed_before) { _old_marking_cycles_completed_before) {
FullGCCount_lock->wait(Mutex::_no_safepoint_check_flag); FullGCCount_lock->wait(Mutex::_no_safepoint_check_flag);
} }
} }

2
hotspot/src/share/vm/gc_implementation/g1/vm_operations_g1.hpp
View File

@ -80,7 +80,7 @@ private:
bool _should_initiate_conc_mark; bool _should_initiate_conc_mark;
bool _should_retry_gc; bool _should_retry_gc;
double _target_pause_time_ms; double _target_pause_time_ms;
unsigned int _full_collections_completed_before; unsigned int _old_marking_cycles_completed_before;
public: public:
VM_G1IncCollectionPause(unsigned int gc_count_before, VM_G1IncCollectionPause(unsigned int gc_count_before,
size_t word_size, size_t word_size,