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

David Holmes david.holmes at oracle.com
Tue Jan 7 09:18:35 UTC 2014


On 7/01/2014 6:16 PM, Tristan Yan wrote:
> Thank you, David
> I fixed copyright and change back sleep.
> println was intended to be left in. This test was failed with timeout,
> printf could help us to detect the value of total_turns_taken and
> expected_turns_taken.
> Please review it again
>
> http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/

Comma after 2014 still missing in copyright.

You need to read total_turns_taken.get() once and use that value in both 
the println and the == test, so that you print the value you actually 
tested.

David

> Regards
> Tristan
>
> On 01/07/2014 03:10 PM, David Holmes wrote:
>> Hi Tristan,
>>
>> On 7/01/2014 4:43 PM, Tristan Yan wrote:
>>> Hi All
>>> Please help to review the code change for JDK-7027502.
>>>
>>> http://cr.openjdk.java.net/~tyan/JDK-7027502/
>>>
>>> Description:
>>> This test was failed in JPRT test but recently we test with same
>>> binaries run, it doesn't fail any more. The intention for this code
>>> change is bringing this test back to normal test and make this test
>>> robust and more informative.  Change includes
>>> 1. Remove this test from ProblemList.
>>> 2. Change static member total_turns_taken into a local variable.
>>>
>>> Please let me know your comment on this.
>>
>>    2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All
>> rights reserved.
>>
>> Correct copyright format should have a space before 2014 and a comma
>> after:
>>
>>  * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights
>> reserved.
>>
>> ---
>>
>> Was this println intended to be left in?
>>
>>  114                 System.out.println("total_turns_taken =" +
>> total_turns_taken +
>>  115                         ";expected_turns_taken =" +
>> expected_turns_taken);
>>  116                 if ( total_turns_taken.get() ==
>> expected_turns_taken ) {
>>
>>
>> You only want to read total_turns_taken once otherwise you may see
>> misleading print outs.
>>
>> ---
>>
>> 119                 /* Create some monitor contention by sleeping with
>> lock */
>>  120                 if ( default_contention_sleep > 0 ) {
>>  121                     System.out.println("Context sleeping, to
>> create contention");
>>  122                     try {
>>  123                         turn.wait((long) default_contention_sleep);
>>  124                     } catch (InterruptedException ignore) { }
>>  125                 }
>>
>> By changing the Thread.sleep to turn.wait you no longer introduce any
>> contention as the wait() will release the monitor.
>>
>> David
>>
>>> Thank you.
>>> Tristan
>



More information about the core-libs-dev mailing list