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

Sangheon Kim sangheki at openjdk.org
Wed Aug 17 01:08:53 UTC 2022


On Tue, 16 Aug 2022 10:15:03 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

>> 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.

Renamed to "GCCPUTime".

> 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)

1. Changed as you suggested. I had same idea but the original data is in seconds. And I wanted to use same unit as log message and avoid conversion. But as you said, the tool gets the benefit.
1-2. Added supported GCs in the description.

2. Removed description field of gcid.
3. Removed 'thread=true'.

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

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



More information about the hotspot-gc-dev mailing list