8152724: Sum of eden before GC and current survivor capacity may be larger than heap size
authortschatzl
Wed, 12 Dec 2018 12:00:02 +0100
changeset 52975 35e2bbea78b2
parent 52974 ddbd9744a3d5
child 52976 21dfea980e23
8152724: Sum of eden before GC and current survivor capacity may be larger than heap size Summary: Limit the maximum survivor size for a given GC to the remaining number of free regions. Reviewed-by: sjohanss, sangheki
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
src/hotspot/share/gc/g1/g1HeapTransition.cpp
src/hotspot/share/gc/g1/g1Policy.cpp
src/hotspot/share/gc/g1/g1Policy.hpp
test/hotspot/jtreg/gc/g1/TestEdenSurvivorLessThanMax.java
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp	Wed Dec 12 15:04:47 2018 +0530
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp	Wed Dec 12 12:00:02 2018 +0100
@@ -1026,6 +1026,9 @@
   // The number of regions that are completely free.
   uint num_free_regions() const { return _hrm.num_free_regions(); }
 
+  // The number of regions that can be allocated into.
+  uint num_free_or_available_regions() const { return num_free_regions() + _hrm.available(); }
+
   MemoryUsage get_auxiliary_data_memory_usage() const {
     return _hrm.get_auxiliary_data_memory_usage();
   }
--- a/src/hotspot/share/gc/g1/g1HeapTransition.cpp	Wed Dec 12 15:04:47 2018 +0530
+++ b/src/hotspot/share/gc/g1/g1HeapTransition.cpp	Wed Dec 12 12:00:02 2018 +0100
@@ -89,7 +89,7 @@
   Data after(_g1_heap);
 
   size_t eden_capacity_length_after_gc = _g1_heap->g1_policy()->young_list_target_length() - after._survivor_length;
-  size_t survivor_capacity_length_after_gc = _g1_heap->g1_policy()->max_survivor_regions();
+  size_t survivor_capacity_length_before_gc = _g1_heap->g1_policy()->max_survivor_regions();
 
   DetailedUsage usage;
   if (log_is_enabled(Trace, gc, heap)) {
@@ -112,7 +112,7 @@
   log_trace(gc, heap)(" Used: 0K, Waste: 0K");
 
   log_info(gc, heap)("Survivor regions: " SIZE_FORMAT "->" SIZE_FORMAT "("  SIZE_FORMAT ")",
-                     _before._survivor_length, after._survivor_length, survivor_capacity_length_after_gc);
+                     _before._survivor_length, after._survivor_length, survivor_capacity_length_before_gc);
   log_trace(gc, heap)(" Used: " SIZE_FORMAT "K, Waste: " SIZE_FORMAT "K",
       usage._survivor_used / K, ((after._survivor_length * HeapRegion::GrainBytes) - usage._survivor_used) / K);
 
--- a/src/hotspot/share/gc/g1/g1Policy.cpp	Wed Dec 12 15:04:47 2018 +0530
+++ b/src/hotspot/share/gc/g1/g1Policy.cpp	Wed Dec 12 12:00:02 2018 +0100
@@ -470,6 +470,10 @@
   // every time we calculate / recalculate the target young length.
   update_survivors_policy();
 
+  assert(max_survivor_regions() + _g1h->num_used_regions() <= _g1h->max_regions(),
+         "Maximum survivor regions %u plus used regions %u exceeds max regions %u",
+         max_survivor_regions(), _g1h->num_used_regions(), _g1h->max_regions());
+
   assert(_g1h->used() == _g1h->recalculate_used(),
          "sanity, used: " SIZE_FORMAT " recalculate_used: " SIZE_FORMAT,
          _g1h->used(), _g1h->recalculate_used());
@@ -899,8 +903,8 @@
   return _young_gen_sizer.adaptive_young_list_length();
 }
 
-size_t G1Policy::desired_survivor_size() const {
-  size_t const survivor_capacity = HeapRegion::GrainWords * _max_survivor_regions;
+size_t G1Policy::desired_survivor_size(uint max_regions) const {
+  size_t const survivor_capacity = HeapRegion::GrainWords * max_regions;
   return (size_t)((((double)survivor_capacity) * TargetSurvivorRatio) / 100);
 }
 
@@ -927,15 +931,22 @@
 void G1Policy::update_survivors_policy() {
   double max_survivor_regions_d =
                  (double) _young_list_target_length / (double) SurvivorRatio;
-  // We use ceiling so that if max_survivor_regions_d is > 0.0 (but
-  // smaller than 1.0) we'll get 1.
-  _max_survivor_regions = (uint) ceil(max_survivor_regions_d);
 
-  _tenuring_threshold = _survivors_age_table.compute_tenuring_threshold(desired_survivor_size());
+  // Calculate desired survivor size based on desired max survivor regions (unconstrained
+  // by remaining heap). Otherwise we may cause undesired promotions as we are
+  // already getting close to end of the heap, impacting performance even more.
+  uint const desired_max_survivor_regions = ceil(max_survivor_regions_d);
+  size_t const survivor_size = desired_survivor_size(desired_max_survivor_regions);
+
+  _tenuring_threshold = _survivors_age_table.compute_tenuring_threshold(survivor_size);
   if (UsePerfData) {
     _policy_counters->tenuring_threshold()->set_value(_tenuring_threshold);
-    _policy_counters->desired_survivor_size()->set_value(desired_survivor_size() * oopSize);
+    _policy_counters->desired_survivor_size()->set_value(survivor_size * oopSize);
   }
+  // The real maximum survivor size is bounded by the number of regions that can
+  // be allocated into.
+  _max_survivor_regions = MIN2(desired_max_survivor_regions,
+                               _g1h->num_free_or_available_regions());
 }
 
 bool G1Policy::force_initial_mark_if_outside_cycle(GCCause::Cause gc_cause) {
--- a/src/hotspot/share/gc/g1/g1Policy.hpp	Wed Dec 12 15:04:47 2018 +0530
+++ b/src/hotspot/share/gc/g1/g1Policy.hpp	Wed Dec 12 12:00:02 2018 +0100
@@ -399,7 +399,7 @@
 
   AgeTable _survivors_age_table;
 
-  size_t desired_survivor_size() const;
+  size_t desired_survivor_size(uint max_regions) const;
 public:
   // Fraction used when predicting how many optional regions to include in
   // the CSet. This fraction of the available time is used for optional regions,
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/gc/g1/TestEdenSurvivorLessThanMax.java	Wed Dec 12 12:00:02 2018 +0100
@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+/*
+ * @test TestEdenSurvivorLessThanMax
+ * @bug 8152724
+ * @summary Check that G1 eden plus survivor max capacity after GC does not exceed maximum number of regions.
+ * @requires vm.gc.G1
+ * @key gc
+ * @library /test/lib
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm -Xbootclasspath/a:. -Xlog:gc+heap=debug -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -XX:+UseG1GC -Xmx64M -Xms64M -Xmn50M -XX:SurvivorRatio=1 TestEdenSurvivorLessThanMax
+ */
+
+import sun.hotspot.WhiteBox;
+
+// The test fills the heap in a way that previous to 8152724 the maximum number of survivor regions
+// for that young gc was higher than there was free space left which is impossible.
+public class TestEdenSurvivorLessThanMax {
+    private static final long BYTES_TO_FILL = 50 * 1024 * 1024;
+
+    private static final WhiteBox WB = WhiteBox.getWhiteBox();
+
+    public static void main(String[] args) throws Exception {
+        Object o = new int[100];
+
+        long objSize = WB.getObjectSize(o);
+
+        // Fill eden to approximately ~90%.
+        int numObjs = (int)((BYTES_TO_FILL / objSize) * 9 / 10);
+        for (int i = 0; i < numObjs; i++) {
+          o = new int[100];
+        }
+
+        WB.youngGC();
+
+        System.out.println(o.toString());
+    }
+}
+