diff src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @ 2030:fb712ff22571

7000559: G1: assertion failure !outer || (full_collections_started == _full_collections_completed + 1) Summary: The concurrent marking thread can complete its operation and increment the full GC counter during a Full GC. This causes the nesting of increments to the start and end of Full GCs that we are expecting to be wrong. the fix is for the marking thread to join the suspendible thread set before incrementing the counter so that it's blocked until the Full GC (or any other safepoint) is finished. The change also includes some minor code cleanup (I renamed a parameter). Reviewed-by: brutisso, ysr
author tonyp
date Tue, 14 Dec 2010 16:19:44 -0500
parents 8df09fb45352
children b03260081e9b
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Thu Dec 09 21:47:58 2010 -0800
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Dec 14 16:19:44 2010 -0500
@@ -1389,7 +1389,7 @@
   }
 
   // Update the number of full collections that have been completed.
-  increment_full_collections_completed(false /* outer */);
+  increment_full_collections_completed(false /* concurrent */);
 
   if (PrintHeapAtGC) {
     Universe::print_heap_after_gc();
@@ -2176,9 +2176,14 @@
      (cause == GCCause::_java_lang_system_gc && ExplicitGCInvokesConcurrent));
 }
 
-void G1CollectedHeap::increment_full_collections_completed(bool outer) {
+void G1CollectedHeap::increment_full_collections_completed(bool concurrent) {
   MonitorLockerEx x(FullGCCount_lock, Mutex::_no_safepoint_check_flag);
 
+  // We assume that if concurrent == true, then the caller is a
+  // concurrent thread that was joined the Suspendible Thread
+  // Set. If there's ever a cheap way to check this, we should add an
+  // 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.
@@ -2192,17 +2197,18 @@
   // behind the number of full collections started.
 
   // This is the case for the inner caller, i.e. a Full GC.
-  assert(outer ||
+  assert(concurrent ||
          (full_collections_started == _full_collections_completed + 1) ||
          (full_collections_started == _full_collections_completed + 2),
-         err_msg("for inner caller: full_collections_started = %u "
+         err_msg("for inner caller (Full GC): full_collections_started = %u "
                  "is inconsistent with _full_collections_completed = %u",
                  full_collections_started, _full_collections_completed));
 
   // This is the case for the outer caller, i.e. the concurrent cycle.
-  assert(!outer ||
+  assert(!concurrent ||
          (full_collections_started == _full_collections_completed + 1),
-         err_msg("for outer caller: full_collections_started = %u "
+         err_msg("for outer caller (concurrent cycle): "
+                 "full_collections_started = %u "
                  "is inconsistent with _full_collections_completed = %u",
                  full_collections_started, _full_collections_completed));
 
@@ -2212,7 +2218,7 @@
   // we wake up any waiters (especially when ExplicitInvokesConcurrent
   // is set) so that if a waiter requests another System.gc() it doesn't
   // incorrectly see that a marking cyle is still in progress.
-  if (outer) {
+  if (concurrent) {
     _cmThread->clear_in_progress();
   }