7001033: assert(gch->gc_cause() == GCCause::_scavenge_alot || !gch->incremental_collection_failed())
authorysr
Tue, 07 Dec 2010 21:55:53 -0800
changeset 7419 263dd4e89b9d
parent 7418 ca00dd5fcae0
child 7420 24071b15dde6
7001033: assert(gch->gc_cause() == GCCause::_scavenge_alot || !gch->incremental_collection_failed()) 7002546: regression on SpecJbb2005 on 7b118 comparing to 7b117 on small heaps Summary: Relaxed assertion checking related to incremental_collection_failed flag to allow for ExplicitGCInvokesConcurrent behaviour where we do not want a failing scavenge to bail to a stop-world collection. Parameterized incremental_collection_will_fail() so we can selectively use, or not use, as appropriate, the statistical prediction at specific use sites. This essentially reverts the scavenge bail-out logic to what it was prior to some recent changes that had inadvertently started using the statistical prediction which can be noisy in the presence of bursty loads. Added some associated verbose non-product debugging messages. Reviewed-by: johnc, tonyp
hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.inline.hpp
hotspot/src/share/vm/memory/collectorPolicy.cpp
hotspot/src/share/vm/memory/defNewGeneration.cpp
hotspot/src/share/vm/memory/genCollectedHeap.cpp
hotspot/src/share/vm/memory/genCollectedHeap.hpp
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Tue Dec 07 21:55:53 2010 -0800
@@ -896,7 +896,7 @@
   size_t available = max_available();
   size_t av_promo  = (size_t)gc_stats()->avg_promoted()->padded_average();
   bool   res = (available >= av_promo) || (available >= max_promotion_in_bytes);
-  if (PrintGC && Verbose) {
+  if (Verbose && PrintGCDetails) {
     gclog_or_tty->print_cr(
       "CMS: promo attempt is%s safe: available("SIZE_FORMAT") %s av_promo("SIZE_FORMAT"),"
       "max_promo("SIZE_FORMAT")",
@@ -1562,8 +1562,8 @@
   GenCollectedHeap* gch = GenCollectedHeap::heap();
   assert(gch->collector_policy()->is_two_generation_policy(),
          "You may want to check the correctness of the following");
-  if (gch->incremental_collection_will_fail()) {
-    if (PrintGCDetails && Verbose) {
+  if (gch->incremental_collection_will_fail(true /* consult_young */)) {
+    if (Verbose && PrintGCDetails) {
       gclog_or_tty->print("CMSCollector: collect because incremental collection will fail ");
     }
     return true;
@@ -1927,7 +1927,7 @@
          "You may want to check the correctness of the following");
   // Inform cms gen if this was due to partial collection failing.
   // The CMS gen may use this fact to determine its expansion policy.
-  if (gch->incremental_collection_will_fail()) {
+  if (gch->incremental_collection_will_fail(false /* don't consult_young */)) {
     assert(!_cmsGen->incremental_collection_failed(),
            "Should have been noticed, reacted to and cleared");
     _cmsGen->set_incremental_collection_failed();
@@ -1936,7 +1936,7 @@
     UseCMSCompactAtFullCollection &&
     ((_full_gcs_since_conc_gc >= CMSFullGCsBeforeCompaction) ||
      GCCause::is_user_requested_gc(gch->gc_cause()) ||
-     gch->incremental_collection_will_fail());
+     gch->incremental_collection_will_fail(true /* consult_young */));
   *should_start_over = false;
   if (clear_all_soft_refs && !*should_compact) {
     // We are about to do a last ditch collection attempt
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.inline.hpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.inline.hpp	Tue Dec 07 21:55:53 2010 -0800
@@ -287,7 +287,7 @@
   // scavenge is done or foreground GC wants to take over collection
   return _collectorState == AbortablePreclean &&
          (_abort_preclean || _foregroundGCIsActive ||
-          GenCollectedHeap::heap()->incremental_collection_will_fail());
+          GenCollectedHeap::heap()->incremental_collection_will_fail(true /* consult_young */));
 }
 
 inline size_t CMSCollector::get_eden_used() const {
--- a/hotspot/src/share/vm/memory/collectorPolicy.cpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/memory/collectorPolicy.cpp	Tue Dec 07 21:55:53 2010 -0800
@@ -685,7 +685,7 @@
       result = expand_heap_and_allocate(size, is_tlab);
     }
     return result;   // could be null if we are out of space
-  } else if (!gch->incremental_collection_will_fail()) {
+  } else if (!gch->incremental_collection_will_fail(false /* don't consult_young */)) {
     // Do an incremental collection.
     gch->do_collection(false            /* full */,
                        false            /* clear_all_soft_refs */,
@@ -693,6 +693,9 @@
                        is_tlab          /* is_tlab */,
                        number_of_generations() - 1 /* max_level */);
   } else {
+    if (Verbose && PrintGCDetails) {
+      gclog_or_tty->print(" :: Trying full because partial may fail :: ");
+    }
     // Try a full collection; see delta for bug id 6266275
     // for the original code and why this has been simplified
     // with from-space allocation criteria modified and
--- a/hotspot/src/share/vm/memory/defNewGeneration.cpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/memory/defNewGeneration.cpp	Tue Dec 07 21:55:53 2010 -0800
@@ -483,16 +483,17 @@
 // so we try to allocate the from-space, too.
 HeapWord* DefNewGeneration::allocate_from_space(size_t size) {
   HeapWord* result = NULL;
-  if (PrintGC && Verbose) {
+  if (Verbose && PrintGCDetails) {
     gclog_or_tty->print("DefNewGeneration::allocate_from_space(%u):"
-                  "  will_fail: %s"
-                  "  heap_lock: %s"
-                  "  free: " SIZE_FORMAT,
-                  size,
-               GenCollectedHeap::heap()->incremental_collection_will_fail() ? "true" : "false",
-               Heap_lock->is_locked() ? "locked" : "unlocked",
-               from()->free());
-    }
+                        "  will_fail: %s"
+                        "  heap_lock: %s"
+                        "  free: " SIZE_FORMAT,
+                        size,
+                        GenCollectedHeap::heap()->incremental_collection_will_fail(false /* don't consult_young */) ?
+                          "true" : "false",
+                        Heap_lock->is_locked() ? "locked" : "unlocked",
+                        from()->free());
+  }
   if (should_allocate_from_space() || GC_locker::is_active_and_needs_gc()) {
     if (Heap_lock->owned_by_self() ||
         (SafepointSynchronize::is_at_safepoint() &&
@@ -534,6 +535,9 @@
   // from this generation, pass on collection; let the next generation
   // do it.
   if (!collection_attempt_is_safe()) {
+    if (Verbose && PrintGCDetails) {
+      gclog_or_tty->print(" :: Collection attempt not safe :: ");
+    }
     gch->set_incremental_collection_failed(); // Slight lie: we did not even attempt one
     return;
   }
@@ -821,6 +825,9 @@
 
 bool DefNewGeneration::collection_attempt_is_safe() {
   if (!to()->is_empty()) {
+    if (Verbose && PrintGCDetails) {
+      gclog_or_tty->print(" :: to is not empty :: ");
+    }
     return false;
   }
   if (_next_gen == NULL) {
@@ -843,10 +850,18 @@
   GenCollectedHeap* gch = GenCollectedHeap::heap();
   if (full) {
     DEBUG_ONLY(seen_incremental_collection_failed = false;)
-    if (!collection_attempt_is_safe()) {
+    if (!collection_attempt_is_safe() && !_eden_space->is_empty()) {
+      if (Verbose && PrintGCDetails) {
+        gclog_or_tty->print("DefNewEpilogue: cause(%s), full, not safe, set_failed, set_alloc_from, clear_seen",
+                            GCCause::to_string(gch->gc_cause()));
+      }
       gch->set_incremental_collection_failed(); // Slight lie: a full gc left us in that state
       set_should_allocate_from_space(); // we seem to be running out of space
     } else {
+      if (Verbose && PrintGCDetails) {
+        gclog_or_tty->print("DefNewEpilogue: cause(%s), full, safe, clear_failed, clear_alloc_from, clear_seen",
+                            GCCause::to_string(gch->gc_cause()));
+      }
       gch->clear_incremental_collection_failed(); // We just did a full collection
       clear_should_allocate_from_space(); // if set
     }
@@ -860,11 +875,20 @@
     // a full collection in between.
     if (!seen_incremental_collection_failed &&
         gch->incremental_collection_failed()) {
+      if (Verbose && PrintGCDetails) {
+        gclog_or_tty->print("DefNewEpilogue: cause(%s), not full, not_seen_failed, failed, set_seen_failed",
+                            GCCause::to_string(gch->gc_cause()));
+      }
       seen_incremental_collection_failed = true;
     } else if (seen_incremental_collection_failed) {
-      assert(gch->gc_cause() == GCCause::_scavenge_alot || !gch->incremental_collection_failed(),
+      if (Verbose && PrintGCDetails) {
+        gclog_or_tty->print("DefNewEpilogue: cause(%s), not full, seen_failed, will_clear_seen_failed",
+                            GCCause::to_string(gch->gc_cause()));
+      }
+      assert(gch->gc_cause() == GCCause::_scavenge_alot ||
+             (gch->gc_cause() == GCCause::_java_lang_system_gc && UseConcMarkSweepGC && ExplicitGCInvokesConcurrent) ||
+             !gch->incremental_collection_failed(),
              "Twice in a row");
-
       seen_incremental_collection_failed = false;
     }
 #endif // ASSERT
--- a/hotspot/src/share/vm/memory/genCollectedHeap.cpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/memory/genCollectedHeap.cpp	Tue Dec 07 21:55:53 2010 -0800
@@ -935,7 +935,7 @@
 void GenCollectedHeap::do_full_collection(bool clear_all_soft_refs,
                                           int max_level) {
   int local_max_level;
-  if (!incremental_collection_will_fail() &&
+  if (!incremental_collection_will_fail(false /* don't consult_young */) &&
       gc_cause() == GCCause::_gc_locker) {
     local_max_level = 0;
   } else {
@@ -951,7 +951,7 @@
   // A scavenge may not have been attempted, or may have
   // been attempted and failed, because the old gen was too full
   if (local_max_level == 0 && gc_cause() == GCCause::_gc_locker &&
-      incremental_collection_will_fail()) {
+      incremental_collection_will_fail(false /* don't consult_young */)) {
     if (PrintGCDetails) {
       gclog_or_tty->print_cr("GC locker: Trying a full collection "
                              "because scavenge failed");
--- a/hotspot/src/share/vm/memory/genCollectedHeap.hpp	Tue Dec 07 16:44:34 2010 -0800
+++ b/hotspot/src/share/vm/memory/genCollectedHeap.hpp	Tue Dec 07 21:55:53 2010 -0800
@@ -477,13 +477,17 @@
   bool no_allocs_since_save_marks(int level);
 
   // Returns true if an incremental collection is likely to fail.
-  bool incremental_collection_will_fail() {
+  // We optionally consult the young gen, if asked to do so;
+  // otherwise we base our answer on whether the previous incremental
+  // collection attempt failed with no corrective action as of yet.
+  bool incremental_collection_will_fail(bool consult_young) {
     // Assumes a 2-generation system; the first disjunct remembers if an
     // incremental collection failed, even when we thought (second disjunct)
     // that it would not.
     assert(heap()->collector_policy()->is_two_generation_policy(),
            "the following definition may not be suitable for an n(>2)-generation system");
-    return incremental_collection_failed() || !get_gen(0)->collection_attempt_is_safe();
+    return incremental_collection_failed() ||
+           (consult_young && !get_gen(0)->collection_attempt_is_safe());
   }
 
   // If a generation bails out of an incremental collection,