Re: RFR (XS) 8214531: HeapMonitorEventOnOffTest.java test fails with "Statistics should be null to begin with"

Fri, 30 Nov 2018 12:23:50 -0800

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 [email protected] <[email protected]> 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).


Thanks,
Jc



--

Thanks,
Jc

Reply via email to