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

[Stefan Johansson]

On Mon, 18 Jan 2021 13:55:06 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:
> 
>   Refactor based on comments

Some comments.

src/hotspot/share/gc/shared/gcTraceSend.cpp line 354:

> 352: 
> 353: void GCLockerTracer::start_gc_locker(const jint jni_lock_count) {
> 354:   assert(SafepointSynchronize::is_at_safepoint(), "sanity");

Maybe add assertions that `_jni_lock_count` and `_stall_count` is 0 to ensure this is not called multiple times for a single event.

src/hotspot/share/gc/shared/gcTraceSend.cpp line 369:

> 367: void GCLockerTracer::report_gc_locker() {
> 368:   Ticks zero;
> 369:   if (_needs_gc_start_timestamp != zero) {

Why is this needed?

src/hotspot/share/gc/shared/gcTraceSend.cpp line 372:

> 370:     EventGCLocker event(UNTIMED);
> 371:     if (event.should_commit()) {
> 372:       event.set_starttime(_needs_gc_start_timestamp);

Shouldn't you also set the endtime using `event.set_endtime(...)`?

src/hotspot/share/gc/shared/gcTraceSend.cpp line 378:

> 376:     }
> 377:     _needs_gc_start_timestamp = zero;
> 378:     _stall_count = 0;

Any reason to not clear `_jni_lock_count`? It would be needed for the assert suggested above.

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

Changes requested by sjohanss (Reviewer).

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