RFR: 8297247: Remark time not added to G1 GarbageCollectorMXBeans [v3]

[Yude Lin]

On Mon, 28 Nov 2022 10:45:08 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Yude Lin has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - An additional test
>>  - More test fixes, including a destruction order problem
>>  - Namings
>
> src/hotspot/share/gc/g1/g1MonitoringSupport.hpp line 132:
> 
>> 130: 
>> 131:   // java.lang.management MemoryManager and MemoryPool support
>> 132:   GCMemoryManager _incremental_memory_manager;
> 
> For consistency's sake it might be useful to change this to `_young_gc_memory_manager`, probably in a separate CR.

Yes, probably also for other memory_managers? in a separate CR.

> test/hotspot/jtreg/gc/g1/TestRemarkCleanupMXBean.java line 73:
> 
>> 71:                                    after + " - " + before + " = " + (after - before));
>> 72:             }
>> 73:         }
> 
> I would prefer if the code would explicitly ask for the concurrent pause GC MXBean and check that its value has been incremented by 2.
> For stability reasons, I recommend excluding that test when run with -Xcomp, because that tends to add tons of GCs.
> I would also prefer if the code used whitebox to explicitly do a concurrent cycle and then check for the expected number of events, although using `System.gc()` is also fine. In that case, you could even tighten the condition to demand four events after two `System.gc()` calls.

done

> test/jdk/com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationContentTest.java line 108:
> 
>> 106:         for(int i = 0; i<10000000; i++) {
>> 107:             data[i%32] = new int[8];
>> 108:         }
> 
> Since the change already adds `WhiteBox`, it would be better to call `wb.youngGC()` instead of this loop.

This test also tests if the allocated objects are collected. So there still needs to be a few allocations. Not sure if we can replace it.

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

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