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

[David Holmes]

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.
>>>