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

[Yasumasa Suenaga]

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/


Could you push?


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
>>>>
>>>
>