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

[Jie Kang]

On Wed, Jun 5, 2019 at 12:21 PM Erik Gahlin <erik.gahlin at oracle.com> wrote:
>
> Frequency can also be used in combination with DataAmount.
>
> For example, the Network Utilization event can have a rate of 100 Mbps
> using the annotations @Frequency @DataAmount(DataAmount.BITS) in
> combination.

For what it's worth, JMC-6348 [1] is an open issue for supporting
multiple content types such as in the NetworkUtilization event. I took
a brief look in the past and I expect work on it in the next few
months but not likely from myself, unfortunately.

[1] https://bugs.openjdk.java.net/browse/JMC-6438

>
> I am thinking of adding two units, Frequency.HERTZ and
> Frequency.PER_SECOND.

I'm a little confused of the difference between the two; isn't Hertz
unit s^-1, also "per second"?


Regards,



>
> Question is, what should be the default?
>
> I'm leaning towards PER_SECOND as it seems more common. This is not for
> JDK 13, as it requires a CSR request and some bake time.
>
> Erik
>
> > 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.
> >
> >
> >>
> >> Regards,
> >>
> > Cheers,
> >
> > Alex
> >
> > [0] http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/2cf5bec8d8ba
>
>