[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (XS) 8214531: HeapMonitorEventOnOffTest.java test fails with "Statistics should be null to b
From: JC Beyler <jcbeyler () google ! com>
Date: 2018-11-30 21:53:34
Message-ID: CAF9BGByyzk-XPPTdYrfR6Xv=9MHHK5-0TWFvLpxNnvjyD6zHdg () mail ! gmail ! com
[Download RAW message or body]
Hi Serguei, thanks!
Done here then:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8214531/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8214531
I've sent it to the submit repo while waiting for a second review :)
Have a great friday!
Jc
On Fri, Nov 30, 2018 at 12:22 PM serguei.spitsyn@oracle.com <
serguei.spitsyn@oracle.com> wrote:
> Hi Jc,
>
> Looks better.
>
> Thanks,
> Serguei
>
>
>
> On 11/30/18 11:48, JC Beyler wrote:
>
> Hi Serguei,
>
> Technically "allocateAndCheckFrames" does enable it internally and the
> comment helps understand that we are testing "sampling on - off - on", no?
>
> I find that without the comments, you now have to understand what allocateAndCheckFrames
> does implicitly.
>
> We could change it to this:
> // By calling allocateAndCheckFrames(), we enabling the notifications
> and check if allocations get sampled again
>
> Does that look better?
>
> Thanks!
> Jc
>
> On Fri, Nov 30, 2018 at 11:12 AM serguei.spitsyn@oracle.com <
> serguei.spitsyn@oracle.com> wrote:
>
>> Hi Jc,
>>
>> It looks good in general.
>> I wonder if this comment is still needed:
>> // Enabling the notification system should start events again.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 11/30/18 09:08, JC Beyler wrote:
>>
>> Hi all,
>>
>> Tiny webrev that removes enabling the sampling system for the
>> HeapMonitorEventOnOffTest before calling the allocateAndCheckFrames method
>> (the allocateAndCheckFrames method enables it internally when no flag is
>> passed to it).
>>
>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8214531/webrev.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214531
>>
>> Thanks,
>> Jc
>>
>>
>>
>
> --
>
> Thanks,
> Jc
>
>
>
--
Thanks,
Jc
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr">Hi Serguei, thanks!<div><br></div><div>Done here \
then:<br></div><div><br></div><div>Webrev: <a \
href="http://cr.openjdk.java.net/~jcbeyler/8214531/webrev.01/">http://cr.openjdk.java.net/~jcbeyler/8214531/webrev.01/</a></div><div>Bug: \
<a href="https://bugs.openjdk.java.net/browse/JDK-8214531" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8214531</a></div><div><br></div><div>I've \
sent it to the submit repo while waiting for a second review \
:)</div><div><br></div><div>Have a great \
friday!<br></div><div>Jc</div></div></div><br><div class="gmail_quote"><div \
dir="ltr">On Fri, Nov 30, 2018 at 12:22 PM <a \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <<a \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_-3594725651702693251moz-cite-prefix">Hi Jc,<br>
<br>
Looks better.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
On 11/30/18 11:48, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Serguei,<br>
<div><br>
</div>
<div>Technically "<span>allocateAndCheckFrames" does enable it
internally and the comment helps understand that we are
testing "sampling on - off - on", no?</span></div>
<div><span><br>
</span></div>
<div><span>I find that without the comments, you now have to
understand what </span><span>allocateAndCheckFrames does
implicitly.</span></div>
<div><span><br>
</span></div>
<div><span>We could change it to this:</span></div>
<div> // By calling allocateAndCheckFrames(), we enabling the
notifications and check if allocations get sampled again<span><br>
</span></div>
<div><br>
</div>
<div>Does that look better?</div>
<div><br>
</div>
<div>Thanks!</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Nov 30, 2018 at 11:12 AM <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> <<a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div class="m_-3594725651702693251m_163393206688762065moz-cite-prefix">Hi \
Jc,<br> <br>
It looks good in general.<br>
I wonder if this comment is still needed:<br>
// Enabling the notification system should start events
again.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 11/30/18 09:08, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>Tiny webrev that removes enabling the sampling
system for the HeapMonitorEventOnOffTest before
calling the <span>allocateAndCheckFrames</span> method
(the <span>allocateAndCheckFrames</span> method
enables it internally when no flag is passed to
it).<br>
<div><br>
</div>
<div>Webrev: <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8214531/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8214531/webrev.00/</a></div> \
<div>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8214531" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8214531</a></div>
<div dir="ltr" \
class="m_-3594725651702693251m_163393206688762065gmail_signature"> <div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-3594725651702693251gmail_signature" \
data-smartmail="gmail_signature"> <div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature" data-smartmail="gmail_signature"><div \
dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic