RFR: 8259808: Add JFR event to detect GC locker stall [v4]

Thomas Schatzl tschatzl at openjdk.java.net
Tue Jan 26 09:52:43 UTC 2021


On Sun, 24 Jan 2021 01:48:57 GMT, Denghui Dong <ddong at openjdk.org> wrote:

>> GC locker will stall the operation of GC, resulting in some Java threads can not continue to run until GC locker is released, thus affecting the response time of the application. Add a JFR event to report this information is helpful to detect this issue.
>> 
>> For the test purpose, I add two Whitebox methods to lock/unlock critical.
>
> Denghui Dong has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add GCLockerTracer::is_started() that makes the logic more clear

Changes requested by tschatzl (Reviewer).

test/jdk/jdk/jfr/event/gc/detailed/TestGCLockerEvent.java line 34:

> 32: import sun.hotspot.WhiteBox;
> 33: 
> 34: /**

This block should be the first thing in the test after the copyright notice.

test/jdk/jdk/jfr/event/gc/detailed/TestGCLockerEvent.java line 116:

> 114:             ts[i] = new Thread(() -> {
> 115:                 STALL_COUNT_SIGNAL.countDown();
> 116:                 for (int j = 0; j < LOOP; j++) {

Since the test already uses WhiteBox, please use whitebox to trigger a gc instead of this dodgy method.

src/hotspot/share/gc/shared/gcLocker.cpp line 101:

> 99:     verify_critical_count();
> 100:     _needs_gc = true;
> 101:     GCLockerTracer::start_gc_locker(_jni_lock_count);

Not really convinced that passing `_jni_lock_count` here gives a lot of information: this is the number of threads in a critical section at the point of the first thread needing a gc. It's probably better than nothing. At least this information should be added to the description of the event (if that is possible).

test/jdk/jdk/jfr/event/gc/detailed/TestGCLockerEvent.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021 Alibaba Group Holding Limited. All Rights Reserved.

Would it be possible to keep with the general format of copyright messages in other code, i.e. "Copyright (c) <year>, <copyright holder>. ..."? I.e. if possible please add a comma after the year.

src/hotspot/share/jfr/metadata/metadata.xml line 1095:

> 1093:   <Event name="GCLocker" category="Java Virtual Machine, GC, Detailed" description="GC Locker Information" label="GC Locker" startTime="true" thread="true" stackTrace="true">
> 1094:     <Field type="uint" name="lockCount" label="Lock Count" />
> 1095:     <Field type="uint" name="stallCount" label="Stall Count" />

Please add descriptions to the fields as mentioned above.

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

PR: https://git.openjdk.java.net/jdk/pull/2088


More information about the hotspot-jfr-dev mailing list