[PATCH] JMC-5706 JavaBlockingRule should report block times

[Joshua Matsuoka]

Hi Jessye,

Looks good! Send the patch with commit message, summary, etc. to one of us
and we can get it pushed for you.

Cheers,

- Josh

On Fri, Jun 21, 2019 at 1:52 PM Alex Macdonald <almacdon at redhat.com> wrote:

> Hi Jessye,
>
> On Thu, Jun 20, 2019 at 3:26 PM Jessye Coleman Shapiro <
> jescolem at redhat.com>
> wrote:
>
> > Hi Alex,
> >
> > On Thu, Jun 20, 2019 at 1:33 PM Alex Macdonald <almacdon at redhat.com>
> > wrote:
> >
> >> Hi Jessye,
> >>
> >> On Tue, Jun 11, 2019 at 2:54 PM Jessye Coleman Shapiro <
> >> jescolem at redhat.com> wrote:
> >>
> >>> Hello,
> >>>
> >>> This patch addresses JMC-5706: JavaBlockingRule should report block
> times
> >>> [0].  I have added the total time a top blocking monitor was blocked on
> >>> to
> >>> the information reported.
> >>>
> >>> I have also edited the 'groupedByThread' list filter condition to
> ensure
> >>> that only 'JavaMonitorEnter' events are being considered when
> determining
> >>> the 'mostBlockedThread'.   Without the filter I was getting a null
> >>> pointer
> >>> exception, as the most common thread has not necessarily been blocked
> and
> >>> therefore may not have a blocking time associated with it - let me know
> >>> if
> >>> this is not the case.
> >>>
> >>> See the attached patch bellow and let me know what you think.
> >>>
> >>
> >> I was looking your JMC-5499 patch at the same time as this one, and I
> >> must have got their bug IDs reversed when I was taking a look through
> them.
> >> As a result, I posted my initial review for this patch on the e-mail
> thread
> >> for JMC-5499 [0]. So I'll copy the notes below:
> >>
> >> The content of this patch look good to me.
> >>
> >> However, there are test failures:
> >>
> >> > TestRulesWithJfr.verifyAllResults:132->verifyRuleResults:159
> >>
> >> because the generated JfrRuleBaseline.xml doesn't match the hardcopy one
> >> used to verify the test. This will need to be updated.
> >>
> >> There's also some empty spaces that can be cleaned up.
> >>
> >> JavaBlockingRule.java
> >> - empty space @ lines 134, 138, 151
> >> - trailing empty space @ line 142
> >>
> >>
> >>>
> >>> Thank you!
> >>>
> >>> Jessye
> >>>
> >>> [0] https://bugs.openjdk.java.net/browse/JMC-5706
> >>
> >>
> >> Cheers,
> >>
> >> Alex
> >>
> >> [0]
> http://mail.openjdk.java.net/pipermail/jmc-dev/2019-June/001146.html
> >>
> >
> > Here is the updated patch with empty and trailing space removed, as well
> > as the TestRulesWithJfr.verifyAllResults test fixed.
> >
>
> Great, thanks. The tests passed and it looks good to me.
>
>
> >
> > Thanks for the help,
> >
> > Jessye
> >
> >
> Cheers,
>
> Alex
>