Review request for JMC-6492: Add unit support for jdk.jfr.Frequency

[Mario Torre]

On 25/06/2019 18:28, Jie Kang wrote:
> On Wed, Jun 5, 2019 at 9:52 AM Alex Macdonald <almacdon at redhat.com> wrote:
>>
>> Hi Jie,
>>
>> On Wed, May 29, 2019 at 9:36 AM Jie Kang <jkang at redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> The attached patch adds a case for contentType "hertz",  annotation
>>> "jdk.jfr.Frequency" used in JFR events like CPUTimeStampCounter, to
>>> use the Hertz unit introduced in JMC-5768.
>>>
>>> The following is a before and after image for the event browser for
>>> CPUTimeStampCounter
>>> https://imgur.com/a/cSVlmOM
>>>
>>> How does it look?
>>
>>
>> This looks good to me.
>>
>> Just a note from playing around with the patch, CPUTimeStampCounter was updated in changeset 2cf5bec8d8ba [0] to use hertz instead of frequency per second. As a result, older recordings will still show their numerical value as per their jfr file, but newer recordings will now correctly use the hz unit.
> 
> Thanks. Any chance this could get another review? It's a tiny patch.

Hi Jie,

Sorry, I missed that you needed a second review. The patch looks fine to
me to go.

Cheers,
Mario

-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898