[10] RFR: 8186018: SA: Monitor Cache Dump in HSDB does not work

[Daniel D. Daugherty]

On 8/9/17 3:14 AM, David Holmes wrote:
> On 9/08/2017 5:52 PM, Yasumasa Suenaga wrote:
>> Hi David,
>>
>>> Copyright format is just two years: first and last, so should be:
>>>
>>> * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights
>>> reserved.
>>
>> Sorry, I've fixed:
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8186018/webrev.02/

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectSynchronizer.java
     No comments.

Thumbs up.

Dan


>>
>>
>> Could you push?
>
> I'll wait for second review. If no one volunteers "overnight" I'll 
> push in my morning.
>
> Cheers,
> David
>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2017-08-09 16:40 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>>> Hi,
>>>
>>> On 9/08/2017 4:55 PM, Yasumasa Suenaga wrote:
>>>>
>>>> Hi David,
>>>>
>>>> I uploaded new webrev:
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8186018/webrev.01/
>>>>
>>>> 2017-08-09 15:32 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>>>>>
>>>>> Hi Yasumasa,
>>>>>
>>>>> On 9/08/2017 3:54 PM, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I tried to check Monitor Cache Dump in HSDB, but it hangs and OOME
>>>>>> occurred.
>>>>>>
>>>>>> How to reproduce:
>>>>>>
>>>>>>      1. Run JShell
>>>>>>      2. Attach HSDB to JShell
>>>>>>         $ jhsdb hsdb --pid <PID>
>>>>>>      3. Select "Monitor Cache Dump" in "Tools" menu.
>>>>>>
>>>>>> ObjectSynchronizer$ObjectMonitorIterator switches monitor block list
>>>>>> when index == 0 at next(). However next() switches blockAddr only.
>>>>>> We should also switch "block".
>>>>>>
>>>>>> I uloaded webrev for this issue. Could you review it?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8186018/webrev.00/
>>>>>
>>>>>
>>>>>
>>>>> The fix looks good. Can you please add a comment inserted at L112:
>>>>>
>>>>> // advance to next block
>>>>>
>>>>> Also update copyright year.
>>>>
>>>>
>>>> Fixed them.
>>>
>>>
>>> Copyright format is just two years: first and last, so should be:
>>>
>>> * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights
>>> reserved.
>>>
>>>>
>>>>> Not sure if we need to do anything explicit about calling next() when
>>>>> we've
>>>>> reached the end of the last block? Current code will throw NPE at 
>>>>> L116,
>>>>> new
>>>>> code will either throw NPE at L116 or perhaps at L113 if 
>>>>> ObjectMonitor
>>>>> constructor doesn't take null.
>>>>
>>>>
>>>> According to JavaDoc [1], next() throws NoSuchElementException if the
>>>> iterator has no more elements.
>>>> So I modified to throw this exception if blockAddr is null.
>>>
>>>
>>> That looks good to me.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> [1]
>>>> https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#next-- 
>>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>
>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>
>>>