RFR 6309226: TEST: java/lang/management/ThreadMXBean/SynchronizationStatistics.java didn't check Thread.sleep

[David Holmes]

On 22/10/2013 9:03 PM, Jaroslav Bachorik wrote:
> On 22.10.2013 09:58, David Holmes wrote:
>> On 21/10/2013 9:55 PM, Jaroslav Bachorik wrote:
>>> Please, review this small patch for a test failing due to the updated
>>> implementation in JDK6.
>>>
>>> Issue:  https://bugs.openjdk.java.net/browse/JDK-6309226
>>> Webrev: http://cr.openjdk.java.net/~jbachorik/6309226/webrev.00/
>>>
>>> The test fails due to the change in mustang where
>>> ThreadMXBean.getThreadInfo().getWaitedTime() and
>>> ThreadMXBean.getThreadInfo().getWaitedCount() include Thread.sleep()
>>> too. Unfortunately, Thread.sleep() is used throughout the test for
>>> synchronization purposes and this breaks the test.
>>>
>>> In the patch I propose to replace Thread.sleep() with busy wait and
>>> hinting the scheduler by Thread.yield(). While not very elegant it
>>> successfully works around inclusion of unknown number of Thread.sleep()s
>>> (they are called in loop).
>>
>> Not elegant and not completely reliable either. Probably adequate on a
>> multi-core system but single-core and with some schedulers it could just
>> be a busy spin.
>
> :/ Ok, so I need to account for the Thread.sleep() calls made outside of
> the test code but still reported by the ThreadMXBean. Not that elegant,
> too, but at least should be reliable.
>
> http://cr.openjdk.java.net/~jbachorik/6309226/webrev.01

Sorry, I can't follow the logic of that test enough to determine whether 
this compensating logic is correct.

Whether this is more reliable depends on whether the 5% tolerance in 
timeRangeCheck is enough to account for all the potential inaccuracies 
in the time measurements. On a lightly loaded system it may be, but 
otherwise ... a context switch after determining the base time and doing 
the sleep could add an arbitrary load and cpu-dependent delay. It might 
be less reliable than the yield approach :(

I can't help wonder whether there is a more explicit synchronization 
mechanism that will avoid the need for goSleep? But that becomes a much 
bigger task to deal with.

I will leave this for the serviceability team to determine the best 
course of action.

Thanks,
David

> -JB-
>
>>
>> David
>>
>>> Thanks,
>>>
>>> -JB-
>