On 23.10.2013 02:40, David Holmes wrote:
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.
It simply adds the number of times and the time spent in sleeping during
calls to goSleep() from the BlockedThread (the one that actually counts).
It seems to be correct - otherwise the test would fail because the
numbers wouldn't match.
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 wonder how would "yield" in busy wait behave on a single core
architecture. I need the second thread to progress while busy-waiting ...
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.
Yes. The only task of this fix is to enable the test to be run even
after Thread.sleep() started to be included in the waited time (sometime
in JDK6 timeframe). I suppose the test was successfully used before the
change and if there are any problems with timing additional issues will
be filed and the test will be redesigned.
For now I would like to keep the change simple and really focus on
making the test executable on JDK7/8.
I will leave this for the serviceability team to determine the best
course of action.
Thanks for valuable comments, anyway.
-JB-
Thanks,
David
-JB-
David
Thanks,
-JB-