8195115: G1 Old Gen MemoryPool CollectionUsage.used values don't reflect mixed GC results
authorphh
Tue, 19 Jun 2018 05:18:49 -0700
changeset 50635 5d3c5af82654
parent 50634 c349d409262a
child 50636 8a18bcdd75ed
8195115: G1 Old Gen MemoryPool CollectionUsage.used values don't reflect mixed GC results Summary: Memory pools can now be optional collection participants, e.g., G1 Old Gen in an incremental collection. Reviewed-by: ehelin, mchung
src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
src/hotspot/share/services/memoryManager.cpp
src/hotspot/share/services/memoryManager.hpp
src/hotspot/share/services/memoryService.cpp
src/hotspot/share/services/memoryService.hpp
test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java
test/hotspot/jtreg/gc/g1/mixedgc/TestOldGenCollectionUsage.java
--- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Tue Jun 19 05:18:49 2018 -0700
@@ -8088,6 +8088,7 @@
     case CMSCollector::InitialMarking:
       initialize(manager /* GC manager */ ,
                  cause   /* cause of the GC */,
+                 true    /* allMemoryPoolsAffected */,
                  true    /* recordGCBeginTime */,
                  true    /* recordPreGCUsage */,
                  false   /* recordPeakUsage */,
@@ -8100,6 +8101,7 @@
     case CMSCollector::FinalMarking:
       initialize(manager /* GC manager */ ,
                  cause   /* cause of the GC */,
+                 true    /* allMemoryPoolsAffected */,
                  false   /* recordGCBeginTime */,
                  false   /* recordPreGCUsage */,
                  false   /* recordPeakUsage */,
@@ -8112,6 +8114,7 @@
     case CMSCollector::Sweeping:
       initialize(manager /* GC manager */ ,
                  cause   /* cause of the GC */,
+                 true    /* allMemoryPoolsAffected */,
                  false   /* recordGCBeginTime */,
                  false   /* recordPreGCUsage */,
                  true    /* recordPeakUsage */,
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Jun 19 05:18:49 2018 -0700
@@ -1737,7 +1737,7 @@
 
   _memory_manager.add_pool(_eden_pool);
   _memory_manager.add_pool(_survivor_pool);
-
+  _memory_manager.add_pool(_old_pool, false /* always_affected_by_gc */);
 }
 
 void G1CollectedHeap::stop() {
@@ -2833,7 +2833,8 @@
     log_info(gc,task)("Using %u workers of %u for evacuation", active_workers, workers()->total_workers());
 
     TraceCollectorStats tcs(g1mm()->incremental_collection_counters());
-    TraceMemoryManagerStats tms(&_memory_manager, gc_cause());
+    TraceMemoryManagerStats tms(&_memory_manager, gc_cause(),
+                                collector_state()->yc_type() == Mixed /* allMemoryPoolsAffected */);
 
     G1HeapTransition heap_transition(this);
     size_t heap_used_bytes_before_gc = used();
--- a/src/hotspot/share/services/memoryManager.cpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/services/memoryManager.cpp	Tue Jun 19 05:18:49 2018 -0700
@@ -43,13 +43,15 @@
   (void)const_cast<instanceOop&>(_memory_mgr_obj = instanceOop(NULL));
 }
 
-void MemoryManager::add_pool(MemoryPool* pool) {
-  assert(_num_pools < MemoryManager::max_num_pools, "_num_pools exceeds the max");
-  if (_num_pools < MemoryManager::max_num_pools) {
-    _pools[_num_pools] = pool;
+int MemoryManager::add_pool(MemoryPool* pool) {
+  int index = _num_pools;
+  assert(index < MemoryManager::max_num_pools, "_num_pools exceeds the max");
+  if (index < MemoryManager::max_num_pools) {
+    _pools[index] = pool;
     _num_pools++;
   }
   pool->add_manager(this);
+  return index;
 }
 
 MemoryManager* MemoryManager::get_code_cache_memory_manager() {
@@ -189,6 +191,15 @@
   delete _current_gc_stat;
 }
 
+void GCMemoryManager::add_pool(MemoryPool* pool) {
+  add_pool(pool, true);
+}
+
+void GCMemoryManager::add_pool(MemoryPool* pool, bool always_affected_by_gc) {
+  int index = MemoryManager::add_pool(pool);
+  _pool_always_affected_by_gc[index] = always_affected_by_gc;
+}
+
 void GCMemoryManager::initialize_gc_stat_info() {
   assert(MemoryService::num_memory_pools() > 0, "should have one or more memory pools");
   _last_gc_stat = new(ResourceObj::C_HEAP, mtGC) GCStatInfo(MemoryService::num_memory_pools());
@@ -230,7 +241,8 @@
 void GCMemoryManager::gc_end(bool recordPostGCUsage,
                              bool recordAccumulatedGCTime,
                              bool recordGCEndTime, bool countCollection,
-                             GCCause::Cause cause) {
+                             GCCause::Cause cause,
+                             bool allMemoryPoolsAffected) {
   if (recordAccumulatedGCTime) {
     _accumulated_timer.stop();
   }
@@ -259,9 +271,11 @@
       MemoryPool* pool = get_memory_pool(i);
       MemoryUsage usage = pool->get_memory_usage();
 
-      // Compare with GC usage threshold
-      pool->set_last_collection_usage(usage);
-      LowMemoryDetector::detect_after_gc_memory(pool);
+      if (allMemoryPoolsAffected || pool_always_affected_by_gc(i)) {
+        // Compare with GC usage threshold
+        pool->set_last_collection_usage(usage);
+        LowMemoryDetector::detect_after_gc_memory(pool);
+      }
     }
   }
 
--- a/src/hotspot/share/services/memoryManager.hpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/services/memoryManager.hpp	Tue Jun 19 05:18:49 2018 -0700
@@ -45,11 +45,12 @@
 class OopClosure;
 
 class MemoryManager : public CHeapObj<mtInternal> {
-private:
+protected:
   enum {
     max_num_pools = 10
   };
 
+private:
   MemoryPool* _pools[max_num_pools];
   int         _num_pools;
 
@@ -67,7 +68,7 @@
     return _pools[index];
   }
 
-  void add_pool(MemoryPool* pool);
+  int add_pool(MemoryPool* pool);
 
   bool is_manager(instanceHandle mh)     { return oopDesc::equals(mh(), _memory_mgr_obj); }
 
@@ -142,11 +143,21 @@
   GCStatInfo*  _current_gc_stat;
   int          _num_gc_threads;
   volatile bool _notification_enabled;
-  const char* _gc_end_message;
+  const char*  _gc_end_message;
+  bool         _pool_always_affected_by_gc[MemoryManager::max_num_pools];
+
 public:
   GCMemoryManager(const char* name, const char* gc_end_message);
   ~GCMemoryManager();
 
+  void add_pool(MemoryPool* pool);
+  void add_pool(MemoryPool* pool, bool always_affected_by_gc);
+
+  bool pool_always_affected_by_gc(int index) {
+    assert(index >= 0 && index < num_memory_pools(), "Invalid index");
+    return _pool_always_affected_by_gc[index];
+  }
+
   void   initialize_gc_stat_info();
 
   bool   is_gc_memory_manager()         { return true; }
@@ -158,7 +169,8 @@
   void   gc_begin(bool recordGCBeginTime, bool recordPreGCUsage,
                   bool recordAccumulatedGCTime);
   void   gc_end(bool recordPostGCUsage, bool recordAccumulatedGCTime,
-                bool recordGCEndTime, bool countCollection, GCCause::Cause cause);
+                bool recordGCEndTime, bool countCollection, GCCause::Cause cause,
+                bool allMemoryPoolsAffected);
 
   void        reset_gc_stat()   { _num_collections = 0; _accumulated_timer.reset(); }
 
--- a/src/hotspot/share/services/memoryService.cpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/services/memoryService.cpp	Tue Jun 19 05:18:49 2018 -0700
@@ -182,10 +182,11 @@
 void MemoryService::gc_end(GCMemoryManager* manager, bool recordPostGCUsage,
                            bool recordAccumulatedGCTime,
                            bool recordGCEndTime, bool countCollection,
-                           GCCause::Cause cause) {
+                           GCCause::Cause cause,
+                           bool allMemoryPoolsAffected) {
   // register the GC end statistics and memory usage
   manager->gc_end(recordPostGCUsage, recordAccumulatedGCTime, recordGCEndTime,
-                  countCollection, cause);
+                  countCollection, cause, allMemoryPoolsAffected);
 }
 
 void MemoryService::oops_do(OopClosure* f) {
@@ -232,6 +233,7 @@
 
 TraceMemoryManagerStats::TraceMemoryManagerStats(GCMemoryManager* gc_memory_manager,
                                                  GCCause::Cause cause,
+                                                 bool allMemoryPoolsAffected,
                                                  bool recordGCBeginTime,
                                                  bool recordPreGCUsage,
                                                  bool recordPeakUsage,
@@ -239,7 +241,8 @@
                                                  bool recordAccumulatedGCTime,
                                                  bool recordGCEndTime,
                                                  bool countCollection) {
-  initialize(gc_memory_manager, cause, recordGCBeginTime, recordPreGCUsage, recordPeakUsage,
+  initialize(gc_memory_manager, cause, allMemoryPoolsAffected,
+             recordGCBeginTime, recordPreGCUsage, recordPeakUsage,
              recordPostGCUsage, recordAccumulatedGCTime, recordGCEndTime,
              countCollection);
 }
@@ -248,6 +251,7 @@
 // the MemoryService
 void TraceMemoryManagerStats::initialize(GCMemoryManager* gc_memory_manager,
                                          GCCause::Cause cause,
+                                         bool allMemoryPoolsAffected,
                                          bool recordGCBeginTime,
                                          bool recordPreGCUsage,
                                          bool recordPeakUsage,
@@ -256,6 +260,7 @@
                                          bool recordGCEndTime,
                                          bool countCollection) {
   _gc_memory_manager = gc_memory_manager;
+  _allMemoryPoolsAffected = allMemoryPoolsAffected;
   _recordGCBeginTime = recordGCBeginTime;
   _recordPreGCUsage = recordPreGCUsage;
   _recordPeakUsage = recordPeakUsage;
@@ -271,5 +276,5 @@
 
 TraceMemoryManagerStats::~TraceMemoryManagerStats() {
   MemoryService::gc_end(_gc_memory_manager, _recordPostGCUsage, _recordAccumulatedGCTime,
-                        _recordGCEndTime, _countCollection, _cause);
+                        _recordGCEndTime, _countCollection, _cause, _allMemoryPoolsAffected);
 }
--- a/src/hotspot/share/services/memoryService.hpp	Tue Jun 19 07:54:11 2018 -0400
+++ b/src/hotspot/share/services/memoryService.hpp	Tue Jun 19 05:18:49 2018 -0700
@@ -102,7 +102,8 @@
   static void gc_end(GCMemoryManager* manager, bool recordPostGCUsage,
                      bool recordAccumulatedGCTime,
                      bool recordGCEndTime, bool countCollection,
-                     GCCause::Cause cause);
+                     GCCause::Cause cause,
+                     bool allMemoryPoolsAffected);
 
   static void oops_do(OopClosure* f);
 
@@ -116,6 +117,7 @@
 class TraceMemoryManagerStats : public StackObj {
 private:
   GCMemoryManager* _gc_memory_manager;
+  bool         _allMemoryPoolsAffected;
   bool         _recordGCBeginTime;
   bool         _recordPreGCUsage;
   bool         _recordPeakUsage;
@@ -128,6 +130,7 @@
   TraceMemoryManagerStats() {}
   TraceMemoryManagerStats(GCMemoryManager* gc_memory_manager,
                           GCCause::Cause cause,
+                          bool allMemoryPoolsAffected = true,
                           bool recordGCBeginTime = true,
                           bool recordPreGCUsage = true,
                           bool recordPeakUsage = true,
@@ -138,6 +141,7 @@
 
   void initialize(GCMemoryManager* gc_memory_manager,
                   GCCause::Cause cause,
+                  bool allMemoryPoolsAffected,
                   bool recordGCBeginTime,
                   bool recordPreGCUsage,
                   bool recordPeakUsage,
--- a/test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java	Tue Jun 19 07:54:11 2018 -0400
+++ b/test/hotspot/jtreg/gc/TestMemoryMXBeansAndPoolsPresence.java	Tue Jun 19 05:18:49 2018 -0700
@@ -86,7 +86,7 @@
     public static void main(String[] args) {
         switch (args[0]) {
             case "G1":
-                test(new GCBeanDescription("G1 Young Generation", new String[] {"G1 Eden Space", "G1 Survivor Space"}),
+                test(new GCBeanDescription("G1 Young Generation", new String[] {"G1 Eden Space", "G1 Survivor Space", "G1 Old Gen"}),
                      new GCBeanDescription("G1 Old Generation",   new String[] {"G1 Eden Space", "G1 Survivor Space", "G1 Old Gen"}));
                 break;
             case "CMS":
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/gc/g1/mixedgc/TestOldGenCollectionUsage.java	Tue Jun 19 05:18:49 2018 -0700
@@ -0,0 +1,221 @@
+/*
+ * 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 TestOldGenCollectionUsage.java
+ * @bug 8195115
+ * @summary G1 Old Gen's CollectionUsage.used is zero after mixed GC which is incorrect
+ * @key gc
+ * @requires vm.gc.G1
+ * @requires vm.opt.MaxGCPauseMillis == "null"
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * @modules java.management
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UseG1GC -XX:+UnlockExperimentalVMOptions -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -verbose:gc -XX:SurvivorRatio=1 -Xmx12m -Xms12m -XX:MaxTenuringThreshold=1 -XX:InitiatingHeapOccupancyPercent=100 -XX:-G1UseAdaptiveIHOP -XX:G1MixedGCCountTarget=4 -XX:MaxGCPauseMillis=30000 -XX:G1HeapRegionSize=1m -XX:G1HeapWastePercent=0 -XX:G1MixedGCLiveThresholdPercent=100 TestOldGenCollectionUsage
+ */
+
+import jdk.test.lib.Asserts;
+import sun.hotspot.WhiteBox;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Collections;
+
+import java.lang.management.*;
+
+// 8195115 says that for the "G1 Old Gen" MemoryPool, CollectionUsage.used
+// is zero for G1 after a mixed collection, which is incorrect.
+
+public class TestOldGenCollectionUsage {
+
+    private String poolName = "G1 Old Gen";
+    private String collectorName = "G1 Young Generation";
+
+    public static void main(String [] args) throws Exception {
+        TestOldGenCollectionUsage t = new TestOldGenCollectionUsage();
+        t.run();
+    }
+
+    public TestOldGenCollectionUsage() {
+        System.out.println("Monitor G1 Old Gen pool with G1 Young Generation collector.");
+    }
+
+    public void run() {
+        // Find memory pool and collector
+        List<MemoryPoolMXBean> pools = ManagementFactory.getMemoryPoolMXBeans();
+        MemoryPoolMXBean pool = null;
+        boolean foundPool = false;
+        for (int i = 0; i < pools.size(); i++) {
+            pool = pools.get(i);
+            String name = pool.getName();
+            if (name.contains(poolName)) {
+                System.out.println("Found pool: " + name);
+                foundPool = true;
+                break;
+            }
+        }
+        if (!foundPool) {
+            throw new RuntimeException(poolName + " not found, test with -XX:+UseG1GC");
+        }
+
+        List<GarbageCollectorMXBean> collectors = ManagementFactory.getGarbageCollectorMXBeans();
+        GarbageCollectorMXBean collector = null;
+        boolean foundCollector = false;
+        for (int i = 0; i < collectors.size(); i++) {
+            collector = collectors.get(i);
+            String name = collector.getName();
+            if (name.contains(collectorName)) {
+                System.out.println("Found collector: " + name);
+                foundCollector = true;
+                break;
+            }
+        }
+        if (!foundCollector) {
+            throw new RuntimeException(collectorName + " not found, test with -XX:+UseG1GC");
+        }
+
+        MixedGCProvoker gcProvoker = new MixedGCProvoker();
+        gcProvoker.allocateOldObjects();
+
+        // Verify no non-zero result was stored
+        long usage = pool.getCollectionUsage().getUsed();
+        System.out.println(poolName + ": usage after GC = " + usage);
+        if (usage > 0) {
+            throw new RuntimeException("Premature mixed collections(s)");
+        }
+
+        // Verify that collections were done
+        long collectionCount = collector.getCollectionCount();
+        System.out.println(collectorName + ": collection count = "
+                           + collectionCount);
+        long collectionTime = collector.getCollectionTime();
+        System.out.println(collectorName + ": collection time  = "
+                           + collectionTime);
+        if (collectionCount <= 0) {
+            throw new RuntimeException("Collection count <= 0");
+        }
+        if (collectionTime <= 0) {
+            throw new RuntimeException("Collector has not run");
+        }
+
+        gcProvoker.provokeMixedGC();
+
+        usage = pool.getCollectionUsage().getUsed();
+        System.out.println(poolName + ": usage after GC = " + usage);
+        if (usage <= 0) {
+            throw new RuntimeException(poolName + " found with zero usage");
+        }
+
+        long newCollectionCount = collector.getCollectionCount();
+        System.out.println(collectorName + ": collection count = "
+                           + newCollectionCount);
+        long newCollectionTime = collector.getCollectionTime();
+        System.out.println(collectorName + ": collection time  = "
+                           + newCollectionTime);
+        if (newCollectionCount <= collectionCount) {
+            throw new RuntimeException("No new collection");
+        }
+        if (newCollectionTime <= collectionTime) {
+            throw new RuntimeException("Collector has not run some more");
+        }
+
+        System.out.println("Test passed.");
+    }
+
+    /**
+     * Utility class to guarantee a mixed GC. The class allocates several arrays and
+     * promotes them to the oldgen. After that it tries to provoke mixed GC by
+     * allocating new objects.
+     *
+     * The necessary condition for guaranteed mixed GC is running MixedGCProvoker is
+     * running in VM with the following flags: -XX:MaxTenuringThreshold=1 -Xms12M
+     * -Xmx12M -XX:G1MixedGCLiveThresholdPercent=100 -XX:G1HeapWastePercent=0
+     * -XX:G1HeapRegionSize=1m
+     */
+    public class MixedGCProvoker {
+        private final WhiteBox WB = WhiteBox.getWhiteBox();
+        private final List<byte[]> liveOldObjects = new ArrayList<>();
+        private final List<byte[]> newObjects = new ArrayList<>();
+
+        public static final int ALLOCATION_SIZE = 20000;
+        public static final int ALLOCATION_COUNT = 15;
+
+        public void allocateOldObjects() {
+            List<byte[]> deadOldObjects = new ArrayList<>();
+            // Allocates buffer and promotes it to the old gen. Mix live and dead old
+            // objects
+            for (int i = 0; i < ALLOCATION_COUNT; ++i) {
+                liveOldObjects.add(new byte[ALLOCATION_SIZE * 5]);
+                deadOldObjects.add(new byte[ALLOCATION_SIZE * 5]);
+            }
+
+            // Do two young collections, MaxTenuringThreshold=1 will force promotion.
+            // G1HeapRegionSize=1m guarantees that old gen regions will be filled.
+            WB.youngGC();
+            WB.youngGC();
+            // Check it is promoted & keep alive
+            Asserts.assertTrue(WB.isObjectInOldGen(liveOldObjects),
+                               "List of the objects is suppose to be in OldGen");
+            Asserts.assertTrue(WB.isObjectInOldGen(deadOldObjects),
+                               "List of the objects is suppose to be in OldGen");
+        }
+
+        /**
+         * Waits until Concurent Mark Cycle finishes
+         * @param wb  Whitebox instance
+         * @param sleepTime sleep time
+         */
+        private void waitTillCMCFinished(int sleepTime) {
+            while (WB.g1InConcurrentMark()) {
+                if (sleepTime > -1) {
+                    try {
+                        Thread.sleep(sleepTime);
+                    } catch (InterruptedException e) {
+                        System.out.println("Got InterruptedException while waiting for ConcMarkCycle to finish");
+                    }
+                }
+            }
+        }
+
+        public void provokeMixedGC() {
+            waitTillCMCFinished(0);
+            WB.g1StartConcMarkCycle();
+            waitTillCMCFinished(0);
+            WB.youngGC();
+
+            System.out.println("Allocating new objects to provoke mixed GC");
+            // Provoke a mixed collection. G1MixedGCLiveThresholdPercent=100
+            // guarantees that full old gen regions will be included.
+            for (int i = 0; i < (ALLOCATION_COUNT * 20); i++) {
+                newObjects.add(new byte[ALLOCATION_SIZE]);
+            }
+            // check that liveOldObjects still alive
+            Asserts.assertTrue(WB.isObjectInOldGen(liveOldObjects),
+                               "List of the objects is suppose to be in OldGen");
+        }
+
+    }
+
+}