RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

[Tristan Yan]

Thanks David
I removed those pointless yield, is it okay you can sponsor this for me?

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.04/ 
<http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.04/>

Regards
Tristan

On 01/09/2014 07:34 PM, David Holmes wrote:
> On 9/01/2014 9:27 PM, David Holmes wrote:
>> On 9/01/2014 9:07 PM, Tristan Yan wrote:
>>> Thank you Paul
>>>
>>> I change turn to local variable as well.
>>>
>>> http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
>>> <http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/>
>>
>> Okay I think I get it now. Both MonitorTest and WaitersTest use the
>> Context class, so if both tests run in the same VM the second test will
>> see the static total_turns_taken and "turn" in the state they were left
>> from the first test - hence the second test will always fail. The bug
>> report suggests making the tests othervm to avoid the problem but
>> instead you have changed from using static state to instance state so
>> that there is no interference.
>>
>> But you can't pass a reference to a simple int, for total_turns_taken,
>> hence you turned it into the only mutable Integer type: AtomicInteger.
>>
>> I'm okay with this final form of the change. othervm would have been
>> simpler :) These are ugly tests even with your changes.
>>
>> BTW:
>>
>>   107         Thread.yield();
>>   108         Thread.sleep(sleepTime);
>>
>> The yield() before the sleep is completely pointless.
>
> Ditto for:
>
>  126                     Thread.yield();
>  127 Thread.sleep((long)default_contention_sleep);
>
> David
> -----
>
>> David
>> -----
>>
>>> I am not sure I understand your second suggestion here,  sum up
>>> thread_turns of each Context(This is a fixed value) doesn't equal
>>> total_turns_taken.
>>>
>>> Regards
>>>
>>> Tristan
>>>
>>> On 01/09/2014 06:28 PM, Paul Sandoz wrote:
>>>
>>>> On Jan 9, 2014, at 10:52 AM, Tristan Yan <tristan.yan at oracle.com> 
>>>> wrote:
>>>>
>>>>> Can someone else give a second review on this.
>>>>>
>>>> In a comment the bug you state: "here total_turns_taken is a static
>>>> variable, it could affect by other tests" I don't quite know under
>>>> what test circumstances that can happen, but if so is the following
>>>> also an issue: 52 private final static TurnChecker turn = new
>>>> TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would
>>>> be for the main loop to sum up thread_turns of each Context, since
>>>> read/writes are all performed in a synchronized block. Paul.
>>>>