<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>