Hi Jc and Serguei,
I have looked at the test fixes and overall they look good. I am a little
concerned with some of the comments
I read that included “Flakiness” and “unlucky” like this comment in
HeapMonitorArrayAllSampledTest.java:
// 10% error ensures a sanity test without becoming flaky.
// Flakiness is due to the fact that this test is dependent on the
sampling rate, which is a
// statistical geometric variable around the sampling rate. This means
that the test could be
// unlucky and not achieve the mean average fast enough for the test case.
So it seems there is some dependencies or variabilities that could cause the
test to fail if it got unlucky.
I understand that it is difficult to test features like this in a reproducible
and reliable way.
My fear is that under certain system loads or configurations these tests may
get “flaky” or “unlucky” and start
causing intermittent failures during test runs.
What kind of testing has been performed for these changes and have you seen and
failures due to “flakiness”
or “unluckiness”?
Thanks,
Jerry
> On May 7, 2018, at 9:49 PM, serguei.spit...@oracle.com wrote:
>
> Hi Jc,
>
> I'll make one more pass through the JVMTI and test fixes.
> However, it would be good if at least one more pair of eyes
> looked at the tests as they are a big part of the fix.
>
> Thanks,
> Serguei
>
>
> On 5/7/18 19:28, JC Beyler wrote:
>> Hi Vladimir,
>>
>> Good catch, I believe it was used before but no longer since we put the
>> heap sampler information directly in the thread structure. I removed it for
>> the next webrev.
>>
>> Could anyone do a review on the JVMTI parts and tests?
>>
>> Thanks a lot for your help!
>> Jc
>>
>> On Mon, May 7, 2018 at 6:31 PM Vladimir Kozlov <vladimir.koz...@oracle.com>
>> wrote:
>>
>>> I did not look on JVMTI part and tests. It looks good to me.
>>>
>>> Where _thread field is used?
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 5/7/18 6:10 PM, JC Beyler wrote:
>>>> Hi all,
>>>>
>>>> With the awesome help of Serguei Spitsyn, we have moved forward on the
>>>> implementation for JEP-331 and have the following webrev for review:
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.18/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8171119
>>>>
>>>> It is based on jdk/jdk so should patch well with a recent tip.
>>>>
>>>> Could we please have some reviews for the webrev? It would be greatly
>>>> appreciated!
>>>>
>>>> Thanks for all your help!
>>>> Jc
>>>>
>
