RFR: 8336299: Improve GCLocker stall diagnostics [v3]

[Stefan Karlsson]

On Tue, 30 Jul 2024 21:53:03 GMT, Neethu Prasad <nprasad at openjdk.org> wrote:

>> **Notes**
>> Adding logs to get more visibility into how fast a thread resumes from allocation stall. 
>> 
>> **Testing**
>> * tier 1, tier 2, hotspot_gc tests.
>> 
>> Example log messages
>> 
>> 1. Last thread exiting. Performing GC after exiting critical section. Thread "main" 0 locked.
>> 
>> 2. Thread exiting critical region Thread "main" 0 locked.
>> 
>> 3. Thread stalled by JNI critical section. Resumed after 586ms. Thread "Thread-0".
>> 
>> 4. Thread blocked to enter critical region. Resumed after 1240ms. Thread "SIGINT handler".
>
> Neethu Prasad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add missing imports and remove unused ones

Hi Neethu,

I glanced at this change and saw a couple of nits that I think would be good to clean out before this gets fully reviewed and integrated.

src/hotspot/share/gc/shared/gcLocker.hpp line 166:

> 164:   GCLockerTimingDebugLogger(const char* log_message);
> 165:   ~GCLockerTimingDebugLogger();
> 166: };

There should be no code after the include guard on line 153. This class should be moved above it. With that said, this class is only used in gcLocker.cpp, so there's really no need to expose it through the gcLocker.hpp file, AFAICT.

Also, note that you are using `/* */` to add a comment about the class, but the rest of the code in this file uses `//`, so I'd prefer to see it changed.

Also note that GitHub complains that your addition lacks a newline at the end of the file. We recently went over the GC code base and fixed issues like that. Maybe there's a way to configure your editor to add one when making edits to the end of a file?

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

Changes requested by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20277#pullrequestreview-2209473053
PR Review Comment: https://git.openjdk.org/jdk/pull/20277#discussion_r1698088045