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

Alex Macdonald almacdon at redhat.com
Tue Jul 16 15:44:18 UTC 2019


On Fri, Jul 12, 2019 at 8:54 AM Mario Torre <neugens at redhat.com> wrote:

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

Great! I can sponsor this patch and will push shortly.


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

Cheers,

Alex


More information about the jmc-dev mailing list