[PATCH] JMC-5706 JavaBlockingRule should report block times

[Alex Macdonald]

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