8298215: gc/g1/TestVerifyGCType.java failed with "Missing expected verification pattern Verifying After GC for: Pause Young (Prepare Mixed): expected true, was false"

Reviewed-by: iwalulya, tschatzl

Author: Kim Barrett

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -190,17 +190,14 @@ uint G1Policy::calculate_desired_eden_length_by_mmu() const {
 void G1Policy::update_young_length_bounds() {
   assert(!Universe::is_fully_initialized() || SafepointSynchronize::is_at_safepoint(), "must be");
   bool for_young_only_phase = collector_state()->in_young_only_phase();
-  // Request at least one eden region to ensure progress.
-  bool after_gc = true;
   update_young_length_bounds(_analytics->predict_pending_cards(for_young_only_phase),
-                             _analytics->predict_rs_length(for_young_only_phase),
-                             after_gc);
+                             _analytics->predict_rs_length(for_young_only_phase));
 }
 
-void G1Policy::update_young_length_bounds(size_t pending_cards, size_t rs_length, bool after_gc) {
+void G1Policy::update_young_length_bounds(size_t pending_cards, size_t rs_length) {
   uint old_young_list_target_length = young_list_target_length();
 
-  uint new_young_list_desired_length = calculate_young_desired_length(pending_cards, rs_length, after_gc);
+  uint new_young_list_desired_length = calculate_young_desired_length(pending_cards, rs_length);
   uint new_young_list_target_length = calculate_young_target_length(new_young_list_desired_length);
   uint new_young_list_max_length = calculate_young_max_length(new_young_list_target_length);
 
@@ -239,7 +236,7 @@ void G1Policy::update_young_length_bounds(size_t pending_cards, size_t rs_length
 // value smaller than what is already allocated or what can actually be allocated.
 // This return value is only an expectation.
 //
-uint G1Policy::calculate_young_desired_length(size_t pending_cards, size_t rs_length, bool after_gc) const {
+uint G1Policy::calculate_young_desired_length(size_t pending_cards, size_t rs_length) const {
   uint min_young_length_by_sizer = _young_gen_sizer.min_desired_young_length();
   uint max_young_length_by_sizer = _young_gen_sizer.max_desired_young_length();
 
@@ -253,18 +250,18 @@ uint G1Policy::calculate_young_desired_length(size_t pending_cards, size_t rs_le
   // Size of the already allocated young gen.
   const uint allocated_young_length = _g1h->young_regions_count();
   // This is the absolute minimum young length that we can return. Ensure that we
-  // don't go below any user-defined minimum bound; but we might have already
-  // allocated more than that for various reasons. In this case, use that.
-  uint absolute_min_young_length = MAX2(allocated_young_length, min_young_length_by_sizer);
+  // don't go below any user-defined minimum bound.  Also, we must have at least
+  // one eden region, to ensure progress. But when revising during the ensuing
+  // mutator phase we might have already allocated more than either of those, in
+  // which case use that.
+  uint absolute_min_young_length = MAX3(min_young_length_by_sizer,
+                                        survivor_length + 1,
+                                        allocated_young_length);
   // Calculate the absolute max bounds. After evac failure or when revising the
   // young length we might have exceeded absolute min length or absolute_max_length,
   // so adjust the result accordingly.
   uint absolute_max_young_length = MAX2(max_young_length_by_sizer, absolute_min_young_length);
 
-  // The absolute minimum young gen length (as provided by the young gen sizer) ensures
-  // that we desire at least one young gen region.
-  assert(absolute_min_young_length > 0, "must be");
-
   uint desired_eden_length_by_mmu = 0;
   uint desired_eden_length_by_pause = 0;
 
@@ -293,28 +290,14 @@ uint G1Policy::calculate_young_desired_length(size_t pending_cards, size_t rs_le
   // Clamp to absolute min/max after we determined desired lengths.
   desired_young_length = clamp(desired_young_length, absolute_min_young_length, absolute_max_young_length);
 
-  // After a garbage collection, make room for at least one eden region (i.e. in addition to
-  // already allocated survivor regions).
-  // This may make desired regions go over absolute maximum length by the heap sizer, however
-  // the immediate full gcs after that young gc (particularly on small heaps) are worse.
-  if (after_gc && (allocated_young_length >= desired_young_length)) {
-   log_trace(gc, ergo, heap)("Young desired length: Desired young region length less than already "
-                              "allocated region length, but requesting one eden region minimum. "
-                              "Expanding desired young length from %u to %u.",
-                              desired_young_length,
-                              allocated_young_length + 1);
-    desired_young_length = allocated_young_length + 1;
-  }
-
-  log_trace(gc, ergo, heap)("Young desired length %u (after gc: %s) "
+  log_trace(gc, ergo, heap)("Young desired length %u "
                             "survivor length %u "
                             "allocated young length %u "
                             "absolute min young length %u "
                             "absolute max young length %u "
                             "desired eden length by mmu %u "
                             "desired eden length by pause %u ",
-                            desired_young_length, BOOL_TO_STR(after_gc),
-                            survivor_length,
+                            desired_young_length, survivor_length,
                             allocated_young_length, absolute_min_young_length,
                             absolute_max_young_length, desired_eden_length_by_mmu,
                             desired_eden_length_by_pause);
@@ -548,10 +531,7 @@ void G1Policy::revise_young_list_target_length(size_t rs_length) {
   size_t thread_buffer_cards = _analytics->predict_dirtied_cards_in_thread_buffers();
   G1DirtyCardQueueSet& dcqs = G1BarrierSet::dirty_card_queue_set();
   size_t pending_cards = dcqs.num_cards() + thread_buffer_cards;
-  // We are only revising young gen length to meet pause time goal, so do not request
-  // at least one eden region for progress. At this point we actually want to run into
-  // a GC soon if young gen is already (too) large.
-  update_young_length_bounds(pending_cards, rs_length, false /* need_one_eden_region */);
+  update_young_length_bounds(pending_cards, rs_length);
 }
 
 void G1Policy::record_full_collection_start() {

src/hotspot/share/gc/g1/g1Policy.hpp

@@ -210,10 +210,9 @@ class G1Policy: public CHeapObj<mtGC> {
 
   // Updates the internal young gen maximum and target and desired lengths.
   // If no parameters are passed, predict pending cards and the RS length using
-  // the prediction model. If after_gc is set, make sure that there is one eden region
-  // available (if there is enough space) to guarantee some progress.
+  // the prediction model.
   void update_young_length_bounds();
-  void update_young_length_bounds(size_t pending_cards, size_t rs_length, bool after_gc);
+  void update_young_length_bounds(size_t pending_cards, size_t rs_length);
 
   // Calculate and return the minimum desired eden length based on the MMU target.
   uint calculate_desired_eden_length_by_mmu() const;
@@ -241,7 +240,7 @@ class G1Policy: public CHeapObj<mtGC> {
 
   // Calculate desired young length based on current situation without taking actually
   // available free regions into account.
-  uint calculate_young_desired_length(size_t pending_cards, size_t rs_length, bool after_gc) const;
+  uint calculate_young_desired_length(size_t pending_cards, size_t rs_length) const;
   // Limit the given desired young length to available free regions.
   uint calculate_young_target_length(uint desired_young_length) const;
   // The GCLocker might cause us to need more regions than the target. Calculate

test/hotspot/jtreg/ProblemList.txt

@@ -80,7 +80,6 @@ gc/stress/gclocker/TestGCLockerWithG1.java 8180622 generic-all
 gc/stress/TestJNIBlockFullGC/TestJNIBlockFullGC.java 8192647 generic-all
 gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java 8241293,8298073 macosx-x64,macosx-aarch64
 gc/stress/TestStressG1Humongous.java 8286554 windows-x64
-gc/g1/TestVerifyGCType.java 8298215 linux-all,macosx-all
 
 #############################################################################
 

test/hotspot/jtreg/gc/g1/TestVerifyGCType.java

@@ -190,18 +190,18 @@ private static void verifyCollection(String name, boolean expectBefore, boolean
         Asserts.assertTrue(ci != null, "Expected GC not found: " + name + "\n" + data);
 
         // Verify Before
-        verifyType(ci, expectBefore, VERIFY_BEFORE);
+        verifyType(ci, expectBefore, VERIFY_BEFORE, data);
         // Verify During
-        verifyType(ci, expectDuring, VERIFY_DURING);
+        verifyType(ci, expectDuring, VERIFY_DURING, data);
         // Verify After
-        verifyType(ci, expectAfter, VERIFY_AFTER);
+        verifyType(ci, expectAfter, VERIFY_AFTER, data);
     }
 
-    private static void verifyType(CollectionInfo ci, boolean shouldExist, String pattern) {
+    private static void verifyType(CollectionInfo ci, boolean shouldExist, String pattern, String data) {
         if (shouldExist) {
-            Asserts.assertTrue(ci.containsVerification(pattern), "Missing expected verification pattern " + pattern + " for: " + ci.getName());
+            Asserts.assertTrue(ci.containsVerification(pattern), "Missing expected verification pattern " + pattern + " for: " + ci.getName() + "\n" + data);
         } else {
-            Asserts.assertFalse(ci.containsVerification(pattern), "Found unexpected verification pattern " + pattern + " for: " + ci.getName());
+            Asserts.assertFalse(ci.containsVerification(pattern), "Found unexpected verification pattern " + pattern + " for: " + ci.getName() + "\n" + data);
         }
     }