RFR 8221507: Implement JFR Events for Shenandoah

[Ken Dobson]

Thank you for your review Aleksey, I've removed the trailing whitespaces
here.

http://cr.openjdk.java.net/~kdobson/53476/webrev

What are my next steps, should I send an exported patch to a committer?

Thanks,

Ken Dobson

On Wed, May 1, 2019 at 8:30 AM Aleksey Shipilev <shade at redhat.com> wrote:

> On 3/29/19 11:08 PM, Roman Kennke wrote:
> >> On Tue, Mar 26, 2019 at 5:16 PM Roman Kennke <rkennke at redhat.com
> <mailto:rkennke at redhat.com>> wrote:
> >>
> >>      > Please review this patch that adds support for two new JFR events
> >>      > ShenandoahHeapRegionStateChange and
> ShenandoahHeapRegionInformation.
> >>      >
> >>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8221507
> >>      > Webrev: http://cr.openjdk.java.net/~kdobson/53476/webrev/
> >>      >
> >>      > The events appears to result in ~2% overhead though there is some
> >>      > uncertainty as to whether some of that is noise.
> >>      >
> >>      > Thanks,
> >>      >
> >>      > Ken Dobson
> >>
> >>     Very nice!
> >>
> >>     The overhead is only present with JFR active, I assume?
> >>
> >> We've noticed that there is some overhead when JFR is included in the
> jdk but not running. This
> >> may be because the event object has to be instantiated at each
> transition whether it's enabled or
> >> not. You can see the exact numbers in the email I previously sent you.
> >
> > Hmm, ok. This only ever happens when regions change state, right? I
> doubt that this is frequent
> > enough to make an actual difference. I suspect that what you see is
> noise. I'm good. Let's also hear
> > Aleksey's opinion though.
>
> It looks fine to me. I ran a few tests of my own, and overheads clearly
> visible with JFR turned on,
> but are not bad. The overheads with JFR turned off are slim to none.
>
> Ken, you might want to clean up trailing whitespaces here:
>
> +void ShenandoahJFRSupport::register_jfr_type_serializers() {
> +  JfrSerializer::register_serializer(TYPE_SHENANDOAHHEAPREGIONSTATE,
> +                                     false,
> +                                     true,
> +                                     new
> ShenandoahHeapRegionStateConstant());
> +}
> +#endif
>
>
> -Aleksey
>
>