RFR (S) 8223441: HeapMonitorStatArrayCorrectnessTest fails due to sampling determinism

[Jean Christophe Beyler]

Yes and actually  the count variable too were missing due to a mishap when
generating the webrev... I had this in my local diff:

   public static void main(String[] args) {
     int sizes[] = {1000, 10000, 100000};
+    double expected = 0;
+    int count = 0;

     for (int currentSize : sizes) {
       System.out.println("Testing size " + currentSize);

Which fixes the issue you were seeing...

Thanks for the  review!
Jc

*From: *Chris Plummer <chris.plummer at oracle.com>
*Date: *Wed, May 8, 2019 at 6:39 PM
*To: *Jean Christophe Beyler, serguei.spitsyn at oracle.com
*Cc: *OpenJDK Serviceability

Hi JC,
>
> I don't see how this builds. You used to have:
>
>   78       double expected = maxIteration;
>
> Now  you have:
>
>   81         expected = maxIteration * count;
>
> And I don't see a declaration of "expected" anywhere.
>
> Other than that the changes look fine.
>
> thanks,
>
> Chris
>
> On 5/8/19 3:53 PM, Jean Christophe Beyler wrote:
>
> Hi all,
>
> Due to this failing pretty heavily a test it seems; is there any way I
> could get a second reviewer so I can hopefully calm the test bug demons?
>
> Thanks!
> Jc
>
> On Wed, May 8, 2019 at 7:49 AM Jean Christophe Beyler <jcbeyler at google.com>
> wrote:
>
>> Hi Serguei,
>>
>> Done & Done; I think 10 instead of 5 will be ok. It only continues
>> running if the sampling interval average has not converged so it should
>> only lengthen degenerate cases.
>>
>> Thanks for the review,
>> Jc
>>
>> On Wed, May 8, 2019 at 2:00 AM serguei.spitsyn at oracle.com <
>> serguei.spitsyn at oracle.com> wrote:
>>
>>> Hi Jc,
>>>
>>> A couple of minor comments.
>>>
>>>   37   private static final int maxCount = 5;
>>>
>>>   Constant names are better to start from a capital letter.
>>>   Also, I'd suggest 10 instead of 5 to make it more reliable.
>>>   But it depend on how much it potentially can make the test to run
>>> slower.
>>>
>>>   94       // If we failed maxCount times, through the exception.
>>>
>>>   A typo: replace "through" with "throw".
>>>
>>>
>>> Otherwise, I'm Okay with this fix.
>>> No need for another webrev.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 5/7/19 19:47, Jean Christophe Beyler wrote:
>>>
>>> Hi all,
>>>
>>> Could I get a review for this test fix:
>>>
>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8223441/webrev.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8223441
>>>
>>> Backstory is:
>>> When I fixed https://bugs.openjdk.java.net/browse/JDK-8215113, I fixed
>>> this failing test in the "actually make the error calculation correct". But
>>> it turns out the test itself failed 7 out of 10 times and when we tested
>>> it, we were apparently "lucky".
>>>
>>> This fix makes the test pass 5k times so I am hopeful it will not show
>>> up again.
>>>
>>> Extra notes:
>>>    - I fixed this by adding a loop that says: if we have not converged
>>> yet, try again at max 5 times; generally we don't converge if the sampling
>>> intervals chosen at the start just have pushed us far enough from the
>>> average we expect that it takes more samples to get us back on track
>>>
>>> Thanks,
>>> Jc
>>>
>>> Ps: FWIW, I'm going to look at the flakyness of the rest of the suite
>>> and see if anything pops up to reduce the test bug noise of this suite
>>>
>>>
>>>
>>
>> --
>>
>> Thanks,
>> Jc
>>
>
>
> --
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190508/84e65c1b/attachment-0001.html>