RFR: 8291753: Add JFR event for GC CPU Time

[Thomas Schatzl]

On Thu, 4 Aug 2022 23:58:02 GMT, Sangheon Kim <sangheki at openjdk.org> wrote:

> Hi all,
> 
> Could I have reviews to add new JFR event for GC CPU time?
> Currently only log message is available for CPU time (user, system, real).
> As there is already GCTraceCPUTime class which is used for a log message, I added GCTracer to deliver the event.
> The log message of CPU time is printed after GC is completed and tried to keep same.
> 
> For G1, manually checked there is not difference.
> 
> For test, I had to add an exception as GCCpuTime will be generated after GC end.
> 
> Testing: tier 1 ~ 3
> 
> Thanks,
> Sangheon

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/shared/gcTraceTime.cpp line 93:

> 91: }
> 92: 
> 93: GCTraceCPUTime::GCTraceCPUTime() { GCTraceCPUTime(nullptr); }

Suggestion:

GCTraceCPUTime::GCTraceCPUTime() : GCTraceCPUTime(nullptr) { }

The way the other constructor is called is wrong afaik. However if serial gc support were added, this constructor could be deleted.

test/jdk/jdk/jfr/event/gc/detailed/TestGCCpuTimeEvent.java line 36:

> 34:  * @key jfr
> 35:  * @requires vm.hasJFR
> 36:  * @requires vm.gc == "G1" | vm.gc == null | vm.gc == "Parallel"

Please also add support for serial gc, it provides tracer classes.
Also I would consider blacklisting non-supporting collectors instead, i.e. ZGC, Shenandoah and Epsilon. That way the missing functionality is eventually found more easily, i.e. by searching for `!= <gc>` instead of finding all tests that exclude that collector.

Also since the test is only run with G1, it would be sufficient to just `@requires G1/null`.

test/jdk/jdk/jfr/event/gc/detailed/TestGCCpuTimeEvent.java line 38:

> 36:  * @requires vm.gc == "G1" | vm.gc == null | vm.gc == "Parallel"
> 37:  * @library /test/lib /test/jdk
> 38:  * @run main/othervm -Xmx32m -XX:+UseG1GC jdk.jfr.event.gc.detailed.TestGCCpuTimeEvent

I think you want separate tests like the `gc/Systemgc.java` test here. This will always run the test with G1 even when initially invoked with the intent to execute the serial gc test.

test/jdk/jdk/jfr/event/gc/detailed/TestGCCpuTimeEvent.java line 56:

> 54:             for (int i = 0; i < 100; i++) {
> 55:                 bytes  = new byte[1024 * 1024];
> 56:             }

Please use Whitebox/WhiteBox.youngGC() to guarantee invocation of a GC. This test currently hinges on the compiler/interpreter not being too clever.

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

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