From 68148a0fa494edfa62a0158188b4593a00854dd0 Mon Sep 17 00:00:00 2001
From: brutisso <unknown>
Date: Fri, 30 Aug 2013 07:31:47 +0200
Subject: [PATCH] 8019902: G1: Use the average heap size rather than the
 minimum heap size to calculate the region size Reviewed-by: tonyp, tschatzl,
 sjohanss

---
 .../vm/gc_implementation/g1/g1CollectorPolicy.cpp   | 10 +++++++++-
 src/share/vm/gc_implementation/g1/heapRegion.cpp    | 13 +++----------
 src/share/vm/gc_implementation/g1/heapRegion.hpp    |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp b/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
index 75497e7c3..178acd26a 100644
--- a/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
+++ b/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
@@ -168,7 +168,15 @@ G1CollectorPolicy::G1CollectorPolicy() :
   // Set up the region size and associated fields. Given that the
   // policy is created before the heap, we have to set this up here,
   // so it's done as soon as possible.
-  HeapRegion::setup_heap_region_size(Arguments::min_heap_size());
+
+  // It would have been natural to pass initial_heap_byte_size() and
+  // max_heap_byte_size() to setup_heap_region_size() but those have
+  // not been set up at this point since they should be aligned with
+  // the region size. So, there is a circular dependency here. We base
+  // the region size on the heap size, but the heap size should be
+  // aligned with the region size. To get around this we use the
+  // unaligned values for the heap.
+  HeapRegion::setup_heap_region_size(InitialHeapSize, MaxHeapSize);
   HeapRegionRemSet::setup_remset_size();
 
   G1ErgoVerbose::initialize();
diff --git a/src/share/vm/gc_implementation/g1/heapRegion.cpp b/src/share/vm/gc_implementation/g1/heapRegion.cpp
index 3a3c99a5e..e66268885 100644
--- a/src/share/vm/gc_implementation/g1/heapRegion.cpp
+++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp
@@ -149,18 +149,11 @@ void HeapRegionDCTOC::walk_mem_region_with_cl(MemRegion mr,
 // many regions in the heap (based on the min heap size).
 #define TARGET_REGION_NUMBER          2048
 
-void HeapRegion::setup_heap_region_size(uintx min_heap_size) {
-  // region_size in bytes
+void HeapRegion::setup_heap_region_size(size_t initial_heap_size, size_t max_heap_size) {
   uintx region_size = G1HeapRegionSize;
   if (FLAG_IS_DEFAULT(G1HeapRegionSize)) {
-    // We base the automatic calculation on the min heap size. This
-    // can be problematic if the spread between min and max is quite
-    // wide, imagine -Xms128m -Xmx32g. But, if we decided it based on
-    // the max size, the region size might be way too large for the
-    // min size. Either way, some users might have to set the region
-    // size manually for some -Xms / -Xmx combos.
-
-    region_size = MAX2(min_heap_size / TARGET_REGION_NUMBER,
+    size_t average_heap_size = (initial_heap_size + max_heap_size) / 2;
+    region_size = MAX2(average_heap_size / TARGET_REGION_NUMBER,
                        (uintx) MIN_REGION_SIZE);
   }
 
diff --git a/src/share/vm/gc_implementation/g1/heapRegion.hpp b/src/share/vm/gc_implementation/g1/heapRegion.hpp
index 68e58d680..097c0b543 100644
--- a/src/share/vm/gc_implementation/g1/heapRegion.hpp
+++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp
@@ -361,7 +361,7 @@ class HeapRegion: public G1OffsetTableContigSpace {
   // CardsPerRegion). All those fields are considered constant
   // throughout the JVM's execution, therefore they should only be set
   // up once during initialization time.
-  static void setup_heap_region_size(uintx min_heap_size);
+  static void setup_heap_region_size(size_t initial_heap_size, size_t max_heap_size);
 
   enum ClaimValues {
     InitialClaimValue          = 0,
-- 
GitLab