8386332: G1: Cleanup pause incorrectly updates old gen MemoryPoolMXBean.getCollectionUsage()

Reviewed-by: ayang, iwalulya

src/hotspot/share/gc/g1/g1MonitoringSupport.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2026, 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
@@ -201,7 +201,7 @@ void G1MonitoringSupport::initialize_serviceability() {
   _full_gc_memory_manager.add_pool(_survivor_space_pool);
   _full_gc_memory_manager.add_pool(_old_gen_pool);
 
-  _conc_gc_memory_manager.add_pool(_old_gen_pool);
+  _conc_gc_memory_manager.add_pool(_old_gen_pool, false /* always_affected_by_gc */);
 
   _young_gc_memory_manager.add_pool(_eden_space_pool);
   _young_gc_memory_manager.add_pool(_survivor_space_pool);
@@ -383,9 +383,10 @@ G1FullGCMonitoringScope::G1FullGCMonitoringScope(G1MonitoringSupport* monitoring
                     "end of major GC") {
 }
 
-G1ConcGCMonitoringScope::G1ConcGCMonitoringScope(G1MonitoringSupport* monitoring_support) :
+G1ConcGCMonitoringScope::G1ConcGCMonitoringScope(G1MonitoringSupport* monitoring_support, bool affects_memory_pools) :
   G1MonitoringScope(monitoring_support,
                     monitoring_support->_conc_collection_counters,
                     &monitoring_support->_conc_gc_memory_manager,
-                    "end of concurrent GC pause") {
+                    "end of concurrent GC pause",
+                    affects_memory_pools) {
 }

src/hotspot/share/gc/g1/g1MonitoringSupport.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2026, 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
@@ -246,6 +246,6 @@ public:
 
 class G1ConcGCMonitoringScope : public G1MonitoringScope {
 public:
-  G1ConcGCMonitoringScope(G1MonitoringSupport* monitoring_support);
+  G1ConcGCMonitoringScope(G1MonitoringSupport* monitoring_support, bool affects_memory_pools);
 };
 #endif // SHARE_GC_G1_G1MONITORINGSUPPORT_HPP

src/hotspot/share/gc/g1/g1VMOperations.cpp

@@ -140,7 +140,7 @@ void VM_G1PauseConcurrent::doit() {
   GCTraceTimePauseTimer       timer(_message, g1h->concurrent_mark()->gc_timer_cm());
   GCTraceTimeDriver           t(&logger, &timer);
 
-  G1ConcGCMonitoringScope monitoring_scope(g1h->monitoring_support());
+  G1ConcGCMonitoringScope monitoring_scope(g1h->monitoring_support(), affects_memory_pools());
   SvcGCMarker sgcm(SvcGCMarker::CONCURRENT);
   IsSTWGCActiveMark x;
 

src/hotspot/share/gc/g1/g1VMOperations.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2026, 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
@@ -87,6 +87,10 @@ protected:
     _gc_id(GCId::current()), _message(message) { }
   virtual void work() = 0;
 
+  // Does this concurrent pause affect the memory pools? If so, update the collectionUsage()
+  // MemoryMXBean for the old gen memory pool (which is the only pool registered for concurrent
+  // pauses).
+  virtual bool affects_memory_pools() const = 0;
 public:
   bool doit_prologue() override;
   void doit_epilogue() override;
@@ -95,6 +99,8 @@ public:
 };
 
 class VM_G1PauseRemark : public VM_G1PauseConcurrent {
+  bool affects_memory_pools() const override { return true; }
+
 public:
   VM_G1PauseRemark() : VM_G1PauseConcurrent("Pause Remark") { }
   VMOp_Type type() const override { return VMOp_G1PauseRemark; }
@@ -102,6 +108,8 @@ public:
 };
 
 class VM_G1PauseCleanup : public VM_G1PauseConcurrent {
+  bool affects_memory_pools() const override { return false; }
+
 public:
   VM_G1PauseCleanup() : VM_G1PauseConcurrent("Pause Cleanup") { }
   VMOp_Type type() const override { return VMOp_G1PauseCleanup; }

test/hotspot/jtreg/gc/g1/TestRemarkCleanupMXBeanCollectionUsage.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2026, 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.
+ */
+
+package gc.g1;
+
+/*
+ * @test TestRemarkCleanupMXBeanCollectionUsage
+ * @bug 8386332
+ * @summary Test that Remark and Cleanup correctly update old pool's getCollectionUsage() bean.
+ * @requires vm.gc.G1
+ * @library /test/lib /
+ * @build   jdk.test.whitebox.WhiteBox
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @run     driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
+ * @run main/othervm -XX:+UseG1GC -Xlog:gc -XX:G1HeapRegionSize=1m -Xms128m -Xmx128m
+ *                   -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   gc.g1.TestRemarkCleanupMXBeanCollectionUsage
+ */
+
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryPoolMXBean;
+import java.lang.management.MemoryUsage;
+import java.lang.ref.Reference;
+
+import jdk.test.lib.Asserts;
+import jdk.test.whitebox.WhiteBox;
+
+public class TestRemarkCleanupMXBeanCollectionUsage {
+    private static WhiteBox wb = WhiteBox.getWhiteBox();
+    private static final int M = 1024 * 1024;
+
+    private static MemoryPoolMXBean findPoolMXBean(String name) throws Exception {
+        for (MemoryPoolMXBean pool : ManagementFactory.getMemoryPoolMXBeans()) {
+            if (pool.getName().equals(name)) {
+                return pool;
+            }
+        }
+        throw new RuntimeException("Pool " + name + " not found.");
+    }
+
+    private static long getCollectionUsageUsedAndPrint(MemoryPoolMXBean pool, String message) {
+        long result = pool.getCollectionUsage().getUsed();
+        System.out.println(message + ": " + result);
+        return result;
+    }
+
+    public static void main(String[] args) throws Exception {
+        Object throwaway = new Object();
+        try {
+            MemoryPoolMXBean oldPool = findPoolMXBean("G1 Old Gen");
+
+            wb.concurrentGCAcquireControl();
+            wb.fullGC();
+            long initialUsage = getCollectionUsageUsedAndPrint(oldPool, "Initial Usage");
+
+            // Allocate something in old gen. CollectionUsage should be updated.
+            throwaway = new byte[M]; // Humongous allocation.
+            wb.fullGC();
+            long afterFirstUsage = getCollectionUsageUsedAndPrint(oldPool, "After first alloc usage");
+            Asserts.assertTrue(afterFirstUsage >= initialUsage + M,
+                               "Full GC should updated collectionUsage. Before " + afterFirstUsage + " after " + initialUsage);
+
+            // Remark pause should update collectionUsage, i.e. the following release of the memory be noticed.
+            throwaway = null;
+            wb.concurrentGCRunTo(wb.G1_AFTER_REBUILD_STARTED);
+            long afterRemarkUsage = getCollectionUsageUsedAndPrint(oldPool, "After Remark usage");
+            Asserts.assertTrue(afterRemarkUsage < afterFirstUsage - M,
+                               "Remark pause should have updated getCollectionUsage(). Before " + afterFirstUsage + " after " + afterRemarkUsage);
+
+            // Cleanup pause should not update collectionUsage, i.e. the following allocation go unnoticed.
+            throwaway = new byte[M];
+            wb.concurrentGCRunTo(wb.G1_AFTER_CLEANUP_STARTED);
+            long afterCleanupUsage = getCollectionUsageUsedAndPrint(oldPool, "After Cleanup usage");
+            Asserts.assertTrue(afterCleanupUsage == afterRemarkUsage,
+                               "Cleanup pause should not update getCollectionUsage(). Before " + afterRemarkUsage + " after " + afterCleanupUsage);
+        } finally {
+            wb.concurrentGCReleaseControl();
+            Reference.reachabilityFence(throwaway);
+        }
+    }
+}