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

[Yasumasa Suenaga]

PING:

Could you review it?

>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.04/

I want to merge this change to jdk 10. So I need a reviewer and sponsor.


Yasumasa


On 2017/11/14 9:58, Yasumasa Suenaga wrote:
> PING:
> Could you review it? We need a reviewer and sponsor.
> 
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.04/
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> 2017-11-09 23:34 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>:
>> Thanks, Jini!
>>
>> I'm waiting for Reviewer and sponsor.
>>
>>
>> Yasumasa
>>
>>
>>
>> On 2017/11/09 23:25, Jini George wrote:
>>>
>>> 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.