[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 8078143: java/lang/management/ThreadMXBean/AllThreadIds.java fails intermittently
From:       David Holmes <david.holmes () oracle ! com>
Date:       2015-05-19 7:44:56
Message-ID: 555AE9F8.2030804 () oracle ! com
[Download RAW message or body]

On 14/05/2015 8:08 PM, Jaroslav Bachorik wrote:
> On 14.5.2015 03:57, David Holmes wrote:
>> On 13/05/2015 9:46 PM, Jaroslav Bachorik wrote:
>>> On 1.5.2015 21:55, Martin Buchholz wrote:
>>>>
>>>>
>>>> On Thu, Apr 30, 2015 at 11:27 AM, Jaroslav Bachorik
>>>> <jaroslav.bachorik@oracle.com <mailto:jaroslav.bachorik@oracle.com>>
>>>> wrote:
>>>>
>>>>     On 30.4.2015 19:18, Martin Buchholz wrote:
>>>>
>>>>         Tests that sleep can almost always be better written some
>>>> other way.
>>>>         In this case, I would prefer busy-waiting with timeout until
>>>> the
>>>>         expected condition becomes true.
>>>>
>>>>
>>>>     The thing is that in case of a real issue with the thread
>>>> counters we
>>>>     a/ would be busy-waiting till the test times out (using an
>>>> arbitrary
>>>>     delay is also problematic due to different performance of different
>>>>     machines running with different configurations)
>>>>
>>>>
>>>> Far less problematic (performance-wise and reliability-wise) than the
>>>> fixed sleep.
>>>>
>>>>     b/ would get a rather confusing message about the test timing
>>>> out at
>>>>     the end
>>>>
>>>>
>>>> You can easily improve the error message.
>>>
>>> Well, not that easily. It is not possible to get a notification when
>>> JTREG decides to timeout the test. So you will get the standard JTREG
>>> message and that's all.
>>>
>>> I was able to modify the test to wait for a given condition and provide
>>> useful messages in case of mismatch and retry. For the price of an
>>> increased complexity. On the other hand, the test should be much more
>>> resilient to timing errors caused by slow setups.
>>>
>>> Updated webrev: http://cr.openjdk.java.net/~jbachorik/8078143/webrev.01
>>
>> Certainly increased complexity - took me quite a while to figure out
>> what it all meant. :) waitForCondition should be waitTillEqual as that
>> is the only condition checked (unless you'd like to make it more complex
>> and pass in a Predicate ;-) ).
>
> I started with Predicate and then realized it brought more problems than
> benefits ... and didn't rename the method afterwords. 'waitTillEquals'
> sounds good.
>
>>
>> I think the Thread.yield would be better as a short sleep
>
> You mean not to use Thread.yield() and do Thread.sleep(1) instead?
>
> I've moved the logical blocks into separate methods to make the test
> more explicit about the steps it takes. Incidentally, this made the
> webrev also more readable ;)
>
> Update: http://cr.openjdk.java.net/~jbachorik/8078143/webrev.02

This seems fine to me.

Thanks,
David


> -JB-
>
>>
>> Thanks,
>> David
>> -----
>>
>>
>>> Thanks,
>>>
>>> -JB-
>
[prev in list] [next in list] [prev in thread] [next in thread]