8019902: G1: Use the average heap size rather than the minimum heap size to calculate the region size
authorbrutisso
Fri, 30 Aug 2013 07:31:47 +0200
changeset 19728 9e1556506d2d
parent 19727 f70e36f468bb
child 19729 0ddd2b7bb9bd
child 19730 4c38872d277e
8019902: G1: Use the average heap size rather than the minimum heap size to calculate the region size Reviewed-by: tonyp, tschatzl, sjohanss
hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp
hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Thu Aug 29 06:53:16 2013 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Fri Aug 30 07:31:47 2013 +0200
@@ -168,7 +168,15 @@
   // 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();
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp	Thu Aug 29 06:53:16 2013 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp	Fri Aug 30 07:31:47 2013 +0200
@@ -149,18 +149,11 @@
 // 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);
   }
 
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp	Thu Aug 29 06:53:16 2013 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp	Fri Aug 30 07:31:47 2013 +0200
@@ -361,7 +361,7 @@
   // 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,