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

[David Holmes]

Thanks Dan.

Yasumasa, I'm pushing the changes.

David

On 10/08/2017 2:52 AM, Daniel D. Daugherty wrote:
> 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
>>>>>>>
>>>>>>
>>>>
>