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 <yasue...@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.
