RFR: 8334147: Shenandoah: Avoid taking lock for disabled free set logging [v8]

[Xiaolong Peng]

On Thu, 27 Jun 2024 15:55:35 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> Hi all, 
>> This pull request propose a fix for the issue https://bugs.openjdk.org/browse/JDK-8334147
>> 
>>> There are multiple places in Shenandoah where we take heap lock for potential free set diagnostics. There is no point in taking that lock if we do not report anything. We should at very least take the lock only when logging is actually needed.
>> 
>> Basically it adds a public method ```ShenandoahFreeSet::log_status_with_heap_lock``` which acquire lock when there is need to print logs, also make the ```log_status``` private since not expect it to be called out of ShenandoahFreeSet class.
>> 
>> <s>The change for ShenandoahFreeSet::rebuild is probably debatable, ShenandoahFreeSet::log_status will acquire lock again after ShenandoahFreeSet::rebuild is executed, hence the metrics/information in log may not be always consistent, but it might be fine. </s>
>> 
>> Additional test:
>> - [x] `make test TEST=hotspot_gc_shenandoah`
>> 
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR   
>>    jtreg:test/hotspot/jtreg:hotspot_gc_shenandoah      259   259     0     0   
>> ==============================
>> TEST SUCCESS
>> 
>> 
>> Best,
>> Xiaolong.
>
> Xiaolong Peng has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Accidentally removed a empty line
>  - Method renaming

I have run test ```make test TEST=jtreg/gc/shenandoah/TestWithLogLevel.java```, verified the GC log, I can find log like this:

[0.188s][info][gc,free     ] Free: 667M, Max: 512K regular, 667M humongous, Frag: 0% external, 0% internal; Used: 0B, Mutator Free: 1334 Collector Reserve: 52736K, Max: 512K; Used: 0B
[0.188s][info][gc,start    ] GC(0) Concurrent reset
[0.188s][info][gc,task     ] GC(0) Using 3 of 6 workers for concurrent reset
[0.188s][info][gc,ergo     ] GC(0) Pacer for Reset. Non-Taxable: 1024M
[0.192s][info][gc          ] GC(0) Concurrent reset 4.207ms
[0.197s][info][gc,start    ] GC(0) Pause Init Mark (unload classes)
[0.197s][info][gc,task     ] GC(0) Using 6 of 6 workers for init marking
[0.198s][info][gc,ergo     ] GC(0) Pacer for Mark. Expected Live: 102M, Free: 667M, Non-Taxable: 68300K, Alloc Tax Rate: 0.2x
[0.198s][info][gc          ] GC(0) Pause Init Mark (unload classes) 1.247ms
[0.198s][info][gc,start    ] GC(0) Concurrent marking roots
[0.198s][info][gc,task     ] GC(0) Using 3 of 6 workers for concurrent marking roots
[0.198s][info][gc          ] GC(0) Concurrent marking roots 0.171ms
[0.198s][info][gc,start    ] GC(0) Concurrent marking (unload classes)
[0.198s][info][gc,task     ] GC(0) Using 3 of 6 workers for concurrent marking
[0.293s][info][gc          ] Cancelling GC: Stopping VM
[0.566s][info][gc          ] GC(0) Concurrent marking (unload classes) 367.435ms
[0.566s][info][gc,free     ] Free: 427M, Max: 512K regular, 427M humongous, Frag: 0% external, 0% internal; Used: 0B, Mutator Free: 854 Collector Reserve: 52736K, Max: 512K; Used: 0B



We don't need to add another test for it.

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

PR Comment: https://git.openjdk.org/jdk/pull/19915#issuecomment-2195445625