[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8214499: SA should follow 8150689
From: Jini George <jini.george () oracle ! com>
Date: 2018-11-30 7:53:17
Message-ID: f395f1e9-1801-fbda-0a2c-4433895f1610 () oracle ! com
[Download RAW message or body]
Yes, Yasumasa. You can push the fix. Thanks!
- Jini.
On 11/30/2018 12:36 PM, Yasumasa Suenaga wrote:
> Hi Jini,
>
> On 2018/11/30 15:15, Jini George wrote:
>>
>> The change looks good to me, Yasumasa. (One minor nit: line 110: 2
>> spaces instead of 4 to align with the rest of the file).
>
> Thanks!
> I will fix it.
>
>
>> Would the second part of this comment,
>>
>> 126 // Print out all monitors that we have locked, or are trying
>> to lock,
>> 127 // including re-locking after being notified or timing out in
>> a wait().
>>
>> continue to be valid now ? (here and in vframe.cpp) ?
>
> "continue" is also available in vframe.cpp:
>
> http://hg.openjdk.java.net/jdk/jdk/file/7d3391e9df19/src/hotspot/share/runtime/vframe.cpp#l213
>
>
> This code will be run if the lock is eliminated and it is in compiled
> frame.
> So I guess it is valid, but I'm not sure.
>
> IMHO it should be fixed as another issue if it is incorrect.
>
> Can I push 8214499 fix?
>
>
> Yasumasa
>
>
>> Thank you,
>> Jini.
>>
>> On 11/30/2018 9:09 AM, Yasumasa Suenaga wrote:
>>> Hi David, Poonam,
>>>
>>> I filed this issue to JBS and uploaded webrev:
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8214499
>>> webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8214499/webrev.00/
>>>
>>> This change works fine on test/hotspot/jtreg/serviceability/sa jtreg
>>> test and submit repo
>>> (mach5-one-ysuenaga-JDK-8214499-20181130-0216-12402).
>>>
>>> Could you review it?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2018年11月30日(金) 10:37 David Holmes <david.holmes@oracle.com>:
>>>>
>>>> Yes please file a bug Yasumasa.
>>>>
>>>> This was an oversight in the fixing of 8150689. I have to remember when
>>>> grepping the code that the SA source is no longer under hotspot :(
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 30/11/2018 5:29 am, Poonam Parhar wrote:
>>>>> Hello Yasumasa,
>>>>>
>>>>> It seems to be a good fix to have in SA. Please file a bug and send in
>>>>> your review request.
>>>>>
>>>>> Thanks,
>>>>> Poonam
>>>>>
>>>>> On 11/29/18 6:29 AM, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Does someone work for adapting SA to the 8150689?
>>>>>> 8150689 fixed not to show incorrect lock information in thread dump.
>>>>>>
>>>>>> jstack code in SA implements which refer to HotSpot implementation.
>>>>>> So it should be fixed as below:
>>>>>>
>>>>>> ----------------------
>>>>>> diff -r 157c1130b46e
>>>>>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java
>>>>>>
>>>>>> Thu Nov 29 07:40:45 2018 +0800
>>>>>> +++
>>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java
>>>>>>
>>>>>> Thu Nov 29 22:52:34 2018 +0900
>>>>>> @@ -1,5 +1,5 @@
>>>>>> /*
>>>>>> - * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All
>>>>>> rights
>>>>>> reserved.
>>>>>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All
>>>>>> rights
>>>>>> reserved.
>>>>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>> *
>>>>>> * This code is free software; you can redistribute it and/or
>>>>>> modify it
>>>>>> @@ -65,14 +65,14 @@
>>>>>> // possible values of java_lang_Thread::ThreadStatus
>>>>>> private static int THREAD_STATUS_NEW;
>>>>>>
>>>>>> - private static int THREAD_STATUS_RUNNABLE;
>>>>>> - private static int THREAD_STATUS_SLEEPING;
>>>>>> - private static int THREAD_STATUS_IN_OBJECT_WAIT;
>>>>>> - private static int THREAD_STATUS_IN_OBJECT_WAIT_TIMED;
>>>>>> - private static int THREAD_STATUS_PARKED;
>>>>>> - private static int THREAD_STATUS_PARKED_TIMED;
>>>>>> - private static int THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER;
>>>>>> - private static int THREAD_STATUS_TERMINATED;
>>>>>> + public static int THREAD_STATUS_RUNNABLE;
>>>>>> + public static int THREAD_STATUS_SLEEPING;
>>>>>> + public static int THREAD_STATUS_IN_OBJECT_WAIT;
>>>>>> + public static int THREAD_STATUS_IN_OBJECT_WAIT_TIMED;
>>>>>> + public static int THREAD_STATUS_PARKED;
>>>>>> + public static int THREAD_STATUS_PARKED_TIMED;
>>>>>> + public static int THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER;
>>>>>> + public static int THREAD_STATUS_TERMINATED;
>>>>>>
>>>>>> // java.util.concurrent.locks.AbstractOwnableSynchronizer fields
>>>>>> private static OopField absOwnSyncOwnerThreadField;
>>>>>> diff -r 157c1130b46e
>>>>>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java
>>>>>>
>>>>>> Thu Nov 29 07:40:45 2018 +0800
>>>>>> +++
>>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java
>>>>>>
>>>>>> Thu Nov 29 22:52:34 2018 +0900
>>>>>> @@ -1,5 +1,5 @@
>>>>>> /*
>>>>>> - * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All
>>>>>> rights
>>>>>> reserved.
>>>>>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All
>>>>>> rights
>>>>>> reserved.
>>>>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>> *
>>>>>> * This code is free software; you can redistribute it and/or
>>>>>> modify it
>>>>>> @@ -106,6 +106,9 @@
>>>>>> StackValue sv = locs.get(0);
>>>>>> if (sv.getType() == BasicType.getTObject()) {
>>>>>> OopHandle o = sv.getObject();
>>>>>> + if
>>>>>> (OopUtilities.threadOopGetThreadStatus(thread.getThreadObj()) ==
>>>>>> OopUtilities.THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER) {
>>>>>> + waitState = "waiting to re-lock in wait()";
>>>>>> + }
>>>>>> printLockedObjectClassName(tty, o, waitState);
>>>>>> }
>>>>>> } else {
>>>>>> @@ -146,13 +149,6 @@
>>>>>> // an inflated monitor that is first on the monitor
>>>>>> list in
>>>>>> // the first frame can block us on a monitor enter.
>>>>>> lockState = identifyLockState(monitor, "waiting to
>>>>>> lock");
>>>>>> - } else if (frameCount != 0) {
>>>>>> - // This is not the first frame so we either own this
>>>>>> monitor
>>>>>> - // or we owned the monitor before and called wait().
>>>>>> Because
>>>>>> - // wait() could have been called on any monitor in a
>>>>>> lower
>>>>>> - // numbered frame on the stack, we have to check all the
>>>>>> - // monitors on the list for this frame.
>>>>>> - lockState = identifyLockState(monitor, "waiting to
>>>>>> re-lock in wait()");
>>>>>> }
>>>>>> printLockedObjectClassName(tty, monitor.owner(),
>>>>>> lockState);
>>>>>> foundFirstMonitor = true;
>>>>>> ----------------------
>>>>>>
>>>>>>
>>>>>> Please tell me if I should file it to JBS and send review request.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic