RFR: 8291753: Add JFR event for GC CPU Time [v2]

[Erik Gahlin]

On Mon, 15 Aug 2022 01:42:14 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
>
> Sangheon Kim has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add Serial GC support and split test.

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

> 496:   </Event>
> 497: 
> 498:   <Event name="GCCpuTime" category="Java Virtual Machine, GC, Detailed" label="GC CPU Time" description="GC CPU Time information in seconds"

Could you change the name to "GCCPUTime". Other events uses capital letters in abbreviations.

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

> 499:     thread="true" stackTrace="false" startTime="false">
> 500:     <Field type="uint" name="gcId" label="GC Identifier" relation="GcId" description="Identifier signifying GC during which the object was promoted" />
> 501:     <Field type="double" name="userTime" label="User Time" />

Could you change the field type to "ulong", convert the value to nanoseconds and set contentType="nanos"? This will allow the output to be properly formatted, i.e. 23 ms, in the jfr-tool. You can also remove the mentioning of "in seconds" in the event description. The content type will allow tools to format the unit. I am also missing supported  GCs in the event  description.

The description of gcId seems strange, which "object"? I think you can skip the description.

The event has thread="true". Is data emitted per thread? If so, it may makes sense to aggregate for all threads. I think most user don't care about which thread the CPU was consumed. Otherwise, set thread="false". 

(If you want per thread for JVM engineers, I think the event name and label should reflect it is per thread and perhaps not enable it in the "detailed" configuration, not "normal", in .jfc files)

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

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