[10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Yasumasa Suenaga-4
Hi all,

Thread dump shows lock objects, however jstack jhsdb and jstack in CLHSDB are not show them.
They are very useful for checking monitors. So jstack mode should show them.

I uploaded webrev. Could you review it?

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

This change prints locked stack looks like:

* jhsdb jstack
----------------
"main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on condition [0x00007f184df31000]
    java.lang.Thread.State: TIMED_WAITING (sleeping)
    JavaThread state: _thread_blocked
  - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
  - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
     - locked <0x00000000dfc13b28> (a java.lang.Class)
----------------

* jstack in CLHSDB
----------------
"main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on condition [0x00007f184df31000]
    java.lang.Thread.State: TIMED_WAITING (sleeping)
    JavaThread state: _thread_blocked
  - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
  - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
     - locked <0x00000000dfc13b28> (a java.lang.Class)
----------------


I cannot access JPRT.
So I need a sponsor.


Thanks,

Yasumasa


Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Jini George
Hi Yasumasa,

This is helpful, but looks like 'locked' is printed even when a thread is waiting for the monitor entry, or has unlocked a lock. For a simple test program, we would have:

"Thread-1" #11 prio=5 tid=0x00007f3938450000 nid=0x6be4 waiting for monitor entry [0x00007f3915010000]
   java.lang.Thread.State: BLOCKED (on object monitor)
   JavaThread state: _thread_blocked
 - ThreadLockTest.run() @bci=5, line=6 (Interpreted frame)
    - locked <0x000000072e96c490> (a java.lang.Class)
 - java.lang.Thread.run() @bci=11, line=844 (Interpreted frame)


"Thread-0" #10 prio=5 tid=0x00007f39383b7800 nid=0x6be3 waiting on condition [0x00007f3915111000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
   JavaThread state: _thread_blocked
 - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
 - ThreadLockTest.run() @bci=8, line=6 (Interpreted frame)
    - locked <0x000000072e96c490> (a java.lang.Class)
 - java.lang.Thread.run() @bci=11, line=844 (Interpreted frame)

The corresponding output from the non SA jstack is as follows:

"Thread-1" #11 prio=5 os_prio=0 tid=0x00007f3938450000 nid=0x6be4 waiting for monitor entry [0x00007f3915010000]
   java.lang.Thread.State: BLOCKED (on object monitor)
   JavaThread state: _thread_blocked
Thread: 0x00007f3938450000  [0x6be4] State: _at_safepoint _has_called_back 0 _at_poll_safepoint 0
   JavaThread state: _thread_blocked
        at ThreadLockTest.run(ThreadLockTest.java:6)
        - waiting to lock <0x000000072e96c490> (a java.lang.Class for ThreadLockTest)
        at java.lang.Thread.run(java.base/Thread.java:844)

   Locked ownable synchronizers:
        - None

"Thread-0" #10 prio=5 os_prio=0 tid=0x00007f39383b7800 nid=0x6be3 waiting on condition [0x00007f3915111000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
   JavaThread state: _thread_blocked
Thread: 0x00007f39383b7800  [0x6be3] State: _at_safepoint _has_called_back 0 _at_poll_safepoint 0
   JavaThread state: _thread_blocked
        at java.lang.Thread.sleep(java.base/Native Method)
        at ThreadLockTest.run(ThreadLockTest.java:6)
        - locked <0x000000072e96c490> (a java.lang.Class for ThreadLockTest)
        at java.lang.Thread.run(java.base/Thread.java:844)

   Locked ownable synchronizers:
        - None

I think the various cases as seen in javaVFrame::print_lock_info_on() would have to be dealt with in SA. It would also be great if you could add a test case or modify an existing test case to test this along with the rest of your changes.  

Thanks,
Jini (Not a (R)eviewer).


On 8/3/2017 8:13 PM, Yasumasa Suenaga wrote:
Hi all,

Thread dump shows lock objects, however jstack jhsdb and jstack in CLHSDB are not show them.
They are very useful for checking monitors. So jstack mode should show them.

I uploaded webrev. Could you review it?

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

This change prints locked stack looks like:

* jhsdb jstack
----------------
"main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on condition [0x00007f184df31000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
   JavaThread state: _thread_blocked
 - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
 - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
    - locked <0x00000000dfc13b28> (a java.lang.Class)
----------------

* jstack in CLHSDB
----------------
"main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on condition [0x00007f184df31000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
   JavaThread state: _thread_blocked
 - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
 - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
    - locked <0x00000000dfc13b28> (a java.lang.Class)
----------------


I cannot access JPRT.
So I need a sponsor.


Thanks,

Yasumasa



Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Yasumasa Suenaga-4
Hi Jini,

Thank you for your suggestion.


> This is helpful, but looks like 'locked' is printed even when a thread
> is waiting for the monitor entry, or has unlocked a lock.

IMHO we have to implement getPendingMonitor() and print_lock_info() in
JavaVFrame.java:

   http://hg.openjdk.java.net/jdk10/hs/hotspot/file/52f2a3a13ed1/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java#l48

I guess it is difficult, so we need more time.


>  It would also be great if you could
> add a test case or modify an existing test case to test this along with
> the rest of your changes.

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?


Thanks,

Yasumasa


On 2017/08/07 16:19, Jini George wrote:

> Hi Yasumasa,
>
> This is helpful, but looks like 'locked' is printed even when a thread
> is waiting for the monitor entry, or has unlocked a lock. For a simple
> test program, we would have:
>
> "Thread-1" #11 prio=5 tid=0x00007f3938450000 nid=0x6be4 waiting for
> monitor entry [0x00007f3915010000]
>      java.lang.Thread.State: BLOCKED (on object monitor)
>      JavaThread state: _thread_blocked
>    - ThreadLockTest.run() @bci=5, line=6 (Interpreted frame)
>       - locked <0x000000072e96c490> (a java.lang.Class)
>    - java.lang.Thread.run() @bci=11, line=844 (Interpreted frame)
>
>
> "Thread-0" #10 prio=5 tid=0x00007f39383b7800 nid=0x6be3 waiting on
> condition [0x00007f3915111000]
>      java.lang.Thread.State: TIMED_WAITING (sleeping)
>      JavaThread state: _thread_blocked
>    - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
>    - ThreadLockTest.run() @bci=8, line=6 (Interpreted frame)
>       - locked <0x000000072e96c490> (a java.lang.Class)
>    - java.lang.Thread.run() @bci=11, line=844 (Interpreted frame)
>
> The corresponding output from the non SA jstack is as follows:
>
> "Thread-1" #11 prio=5 os_prio=0 tid=0x00007f3938450000 nid=0x6be4
> waiting for monitor entry [0x00007f3915010000]
>      java.lang.Thread.State: BLOCKED (on object monitor)
>      JavaThread state: _thread_blocked
> Thread: 0x00007f3938450000  [0x6be4] State: _at_safepoint
> _has_called_back 0 _at_poll_safepoint 0
>      JavaThread state: _thread_blocked
>           at ThreadLockTest.run(ThreadLockTest.java:6)
>           -waiting to lock <0x000000072e96c490> (a java.lang.Class for
> ThreadLockTest)
>           at java.lang.Thread.run(java.base/Thread.java:844)
>
>      Locked ownable synchronizers:
>           - None
>
> "Thread-0" #10 prio=5 os_prio=0 tid=0x00007f39383b7800 nid=0x6be3
> waiting on condition [0x00007f3915111000]
>      java.lang.Thread.State: TIMED_WAITING (sleeping)
>      JavaThread state: _thread_blocked
> Thread: 0x00007f39383b7800  [0x6be3] State: _at_safepoint
> _has_called_back 0 _at_poll_safepoint 0
>      JavaThread state: _thread_blocked
>           at java.lang.Thread.sleep(java.base/Native Method)
>           at ThreadLockTest.run(ThreadLockTest.java:6)
>           - locked <0x000000072e96c490> (a java.lang.Class for
> ThreadLockTest)
>           at java.lang.Thread.run(java.base/Thread.java:844)
>
>      Locked ownable synchronizers:
>           - None
>
> I think the various cases as seen in javaVFrame::print_lock_info_on()
> would have to be dealt with in SA. It would also be great if you could
> add a test case or modify an existing test case to test this along with
> the rest of your changes.
>
> Thanks,
> Jini (Not a (R)eviewer).
>
>
> On 8/3/2017 8:13 PM, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> Thread dump shows lock objects, however jstack jhsdb and jstack in
>> CLHSDB are not show them.
>> They are very useful for checking monitors. So jstack mode should show
>> them.
>>
>> I uploaded webrev. Could you review it?
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8185796/webrev.00/
>>
>> This change prints locked stack looks like:
>>
>> * jhsdb jstack
>> ----------------
>> "main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on
>> condition [0x00007f184df31000]
>>     java.lang.Thread.State: TIMED_WAITING (sleeping)
>>     JavaThread state: _thread_blocked
>>   - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
>>   - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
>>      - locked <0x00000000dfc13b28> (a java.lang.Class)
>> ----------------
>>
>> * jstack in CLHSDB
>> ----------------
>> "main" #1 prio=5 tid=0x00007f1844018800 nid=0xe7f0 waiting on
>> condition [0x00007f184df31000]
>>     java.lang.Thread.State: TIMED_WAITING (sleeping)
>>     JavaThread state: _thread_blocked
>>   - java.lang.Thread.sleep(long) @bci=0 (Interpreted frame)
>>   - LongLock.main(java.lang.String[]) @bci=8, line=4 (Interpreted frame)
>>      - locked <0x00000000dfc13b28> (a java.lang.Class)
>> ----------------
>>
>>
>> I cannot access JPRT.
>> So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Jini George
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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Yasumasa Suenaga-4
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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Yasumasa Suenaga-4
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.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: 8185796: jstack and clhsdb jstack should show lock objects

Yasumasa Suenaga-4
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.
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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.
Reply | Threaded
Open this post in threaded view
|

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

Jini George
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.
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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.
Reply | Threaded
Open this post in threaded view
|

Re: 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.
Reply | Threaded
Open this post in threaded view
|

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

Yasumasa Suenaga-4
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.