6941275: G1: The MemoryPools are incorrectly supported for G1

Summary: The way we were caluclating the max value meant that it might fluctuate during the run and this broke some assumptions inside the MBeans framework. This change sets the max value of each pool to -1, which means undefined according to the spec.
Reviewed-by: mchung, johnc

src/share/vm/services/g1MemoryPool.cpp

@@ -28,12 +28,11 @@
 G1MemoryPoolSuper::G1MemoryPoolSuper(G1CollectedHeap* g1h,
                                      const char* name,
                                      size_t init_size,
-                                     size_t max_size,
                                      bool support_usage_threshold) :
   _g1h(g1h), CollectedMemoryPool(name,
                                  MemoryPool::Heap,
                                  init_size,
-                                 max_size,
+                                 undefined_max(),
                                  support_usage_threshold) {
   assert(UseG1GC, "sanity");
 }
@@ -52,13 +51,6 @@ size_t G1MemoryPoolSuper::eden_space_used(G1CollectedHeap* g1h) {
   return eden_used;
 }
 
-// See the comment at the top of g1MemoryPool.hpp
-size_t G1MemoryPoolSuper::eden_space_max(G1CollectedHeap* g1h) {
-  // This should ensure that it returns a value no smaller than the
-  // region size. Currently, eden_space_committed() guarantees that.
-  return eden_space_committed(g1h);
-}
-
 // See the comment at the top of g1MemoryPool.hpp
 size_t G1MemoryPoolSuper::survivor_space_committed(G1CollectedHeap* g1h) {
   return MAX2(survivor_space_used(g1h), (size_t) HeapRegion::GrainBytes);
@@ -71,13 +63,6 @@ size_t G1MemoryPoolSuper::survivor_space_used(G1CollectedHeap* g1h) {
   return survivor_used;
 }
 
-// See the comment at the top of g1MemoryPool.hpp
-size_t G1MemoryPoolSuper::survivor_space_max(G1CollectedHeap* g1h) {
-  // This should ensure that it returns a value no smaller than the
-  // region size. Currently, survivor_space_committed() guarantees that.
-  return survivor_space_committed(g1h);
-}
-
 // See the comment at the top of g1MemoryPool.hpp
 size_t G1MemoryPoolSuper::old_space_committed(G1CollectedHeap* g1h) {
   size_t committed = overall_committed(g1h);
@@ -99,24 +84,11 @@ size_t G1MemoryPoolSuper::old_space_used(G1CollectedHeap* g1h) {
   return used;
 }
 
-// See the comment at the top of g1MemoryPool.hpp
-size_t G1MemoryPoolSuper::old_space_max(G1CollectedHeap* g1h) {
-  size_t max = overall_max(g1h);
-  size_t eden_max = eden_space_max(g1h);
-  size_t survivor_max = survivor_space_max(g1h);
-  max = subtract_up_to_zero(max, eden_max);
-  max = subtract_up_to_zero(max, survivor_max);
-  max = MAX2(max, (size_t) HeapRegion::GrainBytes);
-  return max;
-}
-
 G1EdenPool::G1EdenPool(G1CollectedHeap* g1h) :
   G1MemoryPoolSuper(g1h,
                     "G1 Eden",
                     eden_space_committed(g1h), /* init_size */
-                    eden_space_max(g1h), /* max_size */
-                    false /* support_usage_threshold */) {
-}
+                    false /* support_usage_threshold */) { }
 
 MemoryUsage G1EdenPool::get_memory_usage() {
   size_t initial_sz = initial_size();
@@ -131,9 +103,7 @@ G1SurvivorPool::G1SurvivorPool(G1CollectedHeap* g1h) :
   G1MemoryPoolSuper(g1h,
                     "G1 Survivor",
                     survivor_space_committed(g1h), /* init_size */
-                    survivor_space_max(g1h), /* max_size */
-                    false /* support_usage_threshold */) {
-}
+                    false /* support_usage_threshold */) { }
 
 MemoryUsage G1SurvivorPool::get_memory_usage() {
   size_t initial_sz = initial_size();
@@ -148,9 +118,7 @@ G1OldGenPool::G1OldGenPool(G1CollectedHeap* g1h) :
   G1MemoryPoolSuper(g1h,
                     "G1 Old Gen",
                     old_space_committed(g1h), /* init_size */
-                    old_space_max(g1h), /* max_size */
-                    true /* support_usage_threshold */) {
-}
+                    true /* support_usage_threshold */) { }
 
 MemoryUsage G1OldGenPool::get_memory_usage() {
   size_t initial_sz = initial_size();

src/share/vm/services/g1MemoryPool.hpp

@@ -74,14 +74,20 @@ class G1CollectedHeap;
 // in the future.
 //
 // 3) Another decision that is again not straightforward is what is
-// the max size that each memory pool can grow to. Right now, we set
-// that the committed size for the eden and the survivors and
-// calculate the old gen max as follows (basically, it's a similar
-// pattern to what we use for the committed space, as described
-// above):
+// the max size that each memory pool can grow to. One way to do this
+// would be to use the committed size for the max for the eden and
+// survivors and calculate the old gen max as follows (basically, it's
+// a similar pattern to what we use for the committed space, as
+// described above):
 //
 //  old_gen_max = overall_max - eden_max - survivor_max
 //
+// Unfortunately, the above makes the max of each pool fluctuate over
+// time and, even though this is allowed according to the spec, it
+// broke several assumptions in the M&M framework (there were cases
+// where used would reach a value greater than max). So, for max we
+// use -1, which means "undefined" according to the spec.
+//
 // 4) Now, there is a very subtle issue with all the above. The
 // framework will call get_memory_usage() on the three pools
 // asynchronously. As a result, each call might get a different value
@@ -125,33 +131,30 @@ protected:
   G1MemoryPoolSuper(G1CollectedHeap* g1h,
                     const char* name,
                     size_t init_size,
-                    size_t max_size,
                     bool support_usage_threshold);
 
   // The reason why all the code is in static methods is so that it
   // can be safely called from the constructors of the subclasses.
 
+  static size_t undefined_max() {
+    return (size_t) -1;
+  }
+
   static size_t overall_committed(G1CollectedHeap* g1h) {
     return g1h->capacity();
   }
   static size_t overall_used(G1CollectedHeap* g1h) {
     return g1h->used_unlocked();
   }
-  static size_t overall_max(G1CollectedHeap* g1h) {
-    return g1h->g1_reserved_obj_bytes();
-  }
 
   static size_t eden_space_committed(G1CollectedHeap* g1h);
   static size_t eden_space_used(G1CollectedHeap* g1h);
-  static size_t eden_space_max(G1CollectedHeap* g1h);
 
   static size_t survivor_space_committed(G1CollectedHeap* g1h);
   static size_t survivor_space_used(G1CollectedHeap* g1h);
-  static size_t survivor_space_max(G1CollectedHeap* g1h);
 
   static size_t old_space_committed(G1CollectedHeap* g1h);
   static size_t old_space_used(G1CollectedHeap* g1h);
-  static size_t old_space_max(G1CollectedHeap* g1h);
 };
 
 // Memory pool that represents the G1 eden.
@@ -163,7 +166,7 @@ public:
     return eden_space_used(_g1h);
   }
   size_t max_size() const {
-    return eden_space_max(_g1h);
+    return undefined_max();
   }
   MemoryUsage get_memory_usage();
 };
@@ -177,7 +180,7 @@ public:
     return survivor_space_used(_g1h);
   }
   size_t max_size() const {
-    return survivor_space_max(_g1h);
+    return undefined_max();
   }
   MemoryUsage get_memory_usage();
 };
@@ -191,7 +194,7 @@ public:
     return old_space_used(_g1h);
   }
   size_t max_size() const {
-    return old_space_max(_g1h);
+    return undefined_max();
   }
   MemoryUsage get_memory_usage();
 };