RFR: 8196521: serviceability/sa/ClhsdbWhere.java fails to find method 'waitForReferencePendingList' in output

[stewartd.qdt]

Thanks David.

-----Original Message-----
From: David Holmes [mailto:david.holmes at oracle.com] 
Sent: Saturday, February 3, 2018 3:58 AM
To: stewartd.qdt <stewartd.qdt at qualcommdatacenter.com>; Sharath Ballal <sharath.ballal at oracle.com>; Jini Susan George <jini.george at oracle.com>; serviceability-dev <serviceability-dev at openjdk.java.net>
Subject: Re: RFR: 8196521: serviceability/sa/ClhsdbWhere.java fails to find method 'waitForReferencePendingList' in output

Hi Daniel, Sharath,

The signal thread won't be found if the test happens to be run with -Xrs, but that seems unlikely.

So seems okay. Your sponsor will need to test on all platforms of course.

Thanks,
David

On 3/02/2018 12:11 AM, stewartd.qdt wrote:
> Hi Sharath,
> 
> Thank you for your comments and insight. I have made the requested 
> changes and the new webrev is at: 
> http://cr.openjdk.java.net/~dstewart/8196521/webrev.01/
> 
> Please have a look and let me know if there are any other changes required. This does pass testing on my system.
> 
> Thanks,
> Daniel
> 
> -----Original Message-----
> From: Sharath Ballal [mailto:sharath.ballal at oracle.com]
> Sent: Friday, February 2, 2018 4:49 AM
> To: David Holmes <david.holmes at oracle.com>; Jini Susan George 
> <jini.george at oracle.com>; stewartd.qdt 
> <stewartd.qdt at qualcommdatacenter.com>; serviceability-dev 
> <serviceability-dev at openjdk.java.net>
> Subject: RE: RFR: 8196521: serviceability/sa/ClhsdbWhere.java fails to 
> find method 'waitForReferencePendingList' in output
> 
> Agree with David.  It looks like the stack will be different on some machines occasionally depending on the load, machine speed etc.
> Hence the following lines in the test
>               "java.lang.ref.Reference",
>               "private static void processPendingReferences",
>               "private static native void waitForReferencePendingList", can be replaced by
> 	Java Stack Trace for Signal Dispatcher
> 	Java Stack Trace for Reference Handler That atlease ensures we are 
> looking for all the threads.
> 
> Thanks,
> Sharath (not a Reviewer)
> 
> 
> -----Original Message-----
> From: David Holmes
> Sent: Thursday, February 01, 2018 1:29 PM
> To: Jini George; stewartd.qdt; serviceability-dev
> Subject: Re: RFR: 8196521: serviceability/sa/ClhsdbWhere.java fails to 
> find method 'waitForReferencePendingList' in output
> 
> On 1/02/2018 5:01 PM, Jini George wrote:
>> Hello Daniel,
>>
>> Your fix looks good to me. You probably could instead add 'remove()'
>> or 'run()' to the list, but I leave it upto you.
> 
> These tests should only be looking for things that are guaranteed to be seen. If waitForReferencePendingList is not there, how do we know that processPendingReferences will be there? We seem to be assuming a quiescent system. We know the threads that will be present, but the only stack entries we should be looking for are those controlled by the test code itself.
> 
> David
> -----
> 
>> Thanks,
>> Jini (Not a (R)eviewer).
>>
>> On 2/1/2018 2:39 AM, stewartd.qdt wrote:
>>> Please review this webrev [1] which attempts to fix a test error in 
>>> serviceability/sa/ClhsdbWhere.java when it is run under an AArch64 
>>> system (not necessarily exclusive to this system, but it was the 
>>> system under test). The bug report [2] provides further details and 
>>> has the jtr report that was generated. Essentially the line 
>>> "waitForReferencePendingList" never actually occurs. The test is 
>>> written such that it is expecting that line.
>>>
>>> This patch simply removes the line from the set of tested lines it 
>>> expects.  I'm not overly happy with this approach as it actually 
>>> removes a test line. However, the test line does not actually appear 
>>> in the output (at least on my system) and I'm not sure if there is 
>>> actually another way of testing for the intent of this line or if it 
>>> doesn't actually have to appear in the output, depending on the 
>>> system. Perhaps the original author could chime in and provide 
>>> further guidance as to the intention of the test.
>>>
>>> I am happy to modify the patch as necessary.
>>>
>>> Regards,
>>>
>>> Daniel Stewart
>>>
>>> [1] - http://cr.openjdk.java.net/~dstewart/8196521/webrev.00/
>>>
>>> [2] - https://bugs.openjdk.java.net/browse/JDK-8196521
>>>