PING: RFR: 8185796: jstack and clhsdb jstack should show lock objects

[Jini George]

Hi Yasumasa,

This looks fine to me.

Thank you,
Jini (Not a Reviewer).

On 11/9/2017 6:55 PM, Yasumasa Suenaga wrote:
> Hi Jini,
> 
> Thank you for your comment!
> I've fixed and uploaded new webrev:
> 
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.04/
> 
>> *
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java 
>>
>>
>> -> Lines 198-212: I feel this commented out code could be removed and
>> replaced by a call to printLockInfo(), though I am not entirely sure as
>> to when this printOn() gets exercised.
> 
> I agree with you to remove these comments.
> They are insufficient to show all locks like a my first webrev [1].
> webrev.04 is implemented to follow HotSpot implementation.
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> [1] http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.00/
> 
> 
> 
> On 2017/11/09 2:19, Jini George wrote:
>> Hi Yasumasa,
>>
>> Your changes look good to me overall. Some nits:
>>
>> *
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/BasicType.java 
>>
>> (lines 138 to 152):
>> -> It would be nice if you could indent the "case" statements.
>>
>> *
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java 
>>
>> -> It would be good if the indentation here for the newly added lines
>> matches that of the rest of the file. (4 spaces instead of 2).
>>
>> *
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java 
>>
>>
>> -> Lines 198-212: I feel this commented out code could be removed and
>> replaced by a call to printLockInfo(), though I am not entirely sure as
>> to when this printOn() gets exercised.
>>
>> * test/hotspot/jtreg/serviceability/sa/TestJhsdbJstackLock.java
>> -> You can remove these lines:
>> import java.util.Scanner;
>> import java.util.stream.Collectors;
>> import java.io.File;
>>
>> Thanks,
>> Jini (Not a Reviewer).
>>
>>
>> On 11/1/2017 6:28 PM, Yasumasa Suenaga wrote:
>>> PING: Could you review and sponsor it?
>>>
>>>> ?? http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.03/
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/10/09 23:19, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> I uploaded new webrev to be adapted to current jdk10/hs:
>>>>
>>>> ?? http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.03/
>>>>
>>>>
>>>> Please review and sponsor it.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2017/09/27 0:31, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> I uploaded new webrev to be adapted to jdk10/hs:
>>>>>
>>>>> ?? http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.02/
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/08/24 22:59, Yasumasa Suenaga wrote:
>>>>>> Thanks Jini!
>>>>>>
>>>>>> I uploaded new webrev:
>>>>>>
>>>>>> ?? http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.01/
>>>>>>
>>>>>> This webrev has been ported print_lock_info() to JavaVFrame.java,
>>>>>> and I've added new testcase for `jhsdb jstack` and jstack command on
>>>>>> `jhsdb clhsdb`.
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/08/24 18:01, Jini George wrote:
>>>>>>> Apologize for the late reply, Yasumasa.
>>>>>>>
>>>>>>>
>>>>>>>> I think so, but I guess it is difficult.
>>>>>>>> For example, test for CLHSDB command is provided as
>>>>>>>> test/serviceability/sa/TestPrintMdo.java .
>>>>>>>> But target process seems to be fixed to "LingeredApp".
>>>>>>>> Can we change it to another program which generates lock 
>>>>>>>> contention?
>>>>>>>
>>>>>>> You can take a look at any of the
>>>>>>> hotspot/test/serviceability/sa/LingeredAppWith*.java files for
>>>>>>> this. The target process does not have to be be fixed to
>>>>>>> LingeredApp -- in these LingeredAppWith* cases, the targets are
>>>>>>> test-specific variations built on top of LingeredApp for ease of
>>>>>>> implementation.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jini.