RFR: 8297247: Add GarbageCollectorMXBean for Remark and Cleanup pause time in G1 [v4]

Albert Mingkun Yang ayang at openjdk.org
Fri Dec 2 14:43:18 UTC 2022


On Tue, 29 Nov 2022 07:55:55 GMT, Yude Lin <duke at openjdk.org> wrote:

>> Hi,
>> 
>> This change uses a G1MonitorScope (covers jstat and GC beans) to replace TraceCollectorStats (only covers jstat) in VM_G1PauseConcurrent::doit(), so that now Remark and Cleanup phase time are both collected by a GC bean.
>> This change adds a third bean "G1 Concurrent GC", after "G1 Young Generation" and "G1 Old generation". This has good continuity, meaning that any current application code who uses "G1 Young Generation" and "G1 Old generation" will see nothing changed, but it will now observe a third bean and can decide what to do with it, e.g., summing all of them up.
>> Also now the gc beans and jstat fields have 1-to-1 correspondance:
>> "G1 Young Generation" -- YGC/YGCT
>> "G1 Old generation" -- FGC/FGCT
>> "G1 Concurrent GC" -- CGC/CGCT
>> 
>> Alternative:
>> I've considered using the existing bean "G1 Old generation", but it's currently used for Full GCs, and I find unifying Full GCs with Remark and Cleanup under the same bean is a little awkward. For example, I couldn't find a proper name for the G1MemoryManager, or a proper "gc end message" for the post-gc notification. Also, in this way, if users have previously hooked "G1 Old generation" to some kind of Full GC alarm, then they might suddenly find a lot of false-positives (caused by Remark and Cleanup). So this is undesirable.
>> 
>> Ran tests:
>> test/hotspot/jtreg:tier1-4
>> test/jdk:tier1
>> test/jdk:jdk_jfr_sanity
>> test/jdk:needs_g1gc
>> -Failure found and fixed: jdk/jfr/event/gc/collection/TestGCEventMixedWithG1ConcurrentMark.java
>> -Failure found and fixed: jdk/jfr/event/gc/collection/TestGCEventMixedWithG1FullCollection.java
>> test/jdk:jdk_management
>> -Failure found and fixed: com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationContentTest.java
>> -Failure found and fixed: com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationTest.java
>> -Failure found and fixed: java/lang/management/MemoryMXBean/MemoryTest.java#id0
>> test/hotspot/jtreg:vmTestbase_vm_gc
>> test/hotspot/jtreg:vmTestbase_nsk_monitoring
>> test/hotspot/jtreg:vmTestbase_vm_gc_misc
>> test/jdk:jdk_jmx
>> test/hotspot/jtreg:hotspot_containers_extended
>
> Yude Lin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Suggested changes to tests

src/hotspot/share/gc/g1/g1MonitoringSupport.cpp line 371:

> 369:   G1MonitoringScope(monitoring_support,
> 370:                     monitoring_support->_incremental_collection_counters,
> 371:                     &monitoring_support->_incremental_memory_manager,

Preexisting: it's slightly odd that one is a pointer and the other not. I wonder if both can be non-pointer.

src/hotspot/share/gc/g1/g1MonitoringSupport.hpp line 254:

> 252: public:
> 253:   G1YoungGCMonitoringScope(G1MonitoringSupport* monitoring_support, bool all_memory_pools_affected);
> 254:   ~G1YoungGCMonitoringScope() { }

I think the empty destructor in child classes can be omitted, and the destructor in base class can probably be `protected`.

test/hotspot/jtreg/gc/g1/TestRemarkCleanupMXBean.java line 31:

> 29:  * @summary Test that Remark and Cleanup are correctly reported by
> 30:  *          a GarbageCollectorMXBean
> 31:  * @requires vm.gc.G1 & vm.compMode != "Xcomp"

Why `vm.compMode != "Xcomp"`?

-------------

PR: https://git.openjdk.org/jdk/pull/11341


More information about the hotspot-gc-dev mailing list