RFR: 8187403: [Unknown generation] is shown in Stack Memory on HSDB

[Yasumasa Suenaga]

Hi Serguet,

Thank you for your comment.

> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegion.java.frames.html
>
>   It seems, there is no reason for renaming 'type' to 't' in the
> initialize() method.

I added new private member "type" as HeapRegionType.
  http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegion.java.udiff.html

So I renamed to "t" to avoid conflict.


> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegionManager.java.frames.html
>
>   89     public HeapRegion addrToRegion(Address addr) {
>   90       return regions().getByAddress(addr);
>   91     }
>
>   A suggestion: replace 'addrToRegion' with 'getByAddress'.
>   It will look similar to the 'heapRegionIterator.'

I've implemented it to follow HotSpot implementation.
  http://hg.openjdk.java.net/jdk10/hs/file/3a45532a1854/src/hotspot/share/gc/g1/heapRegionManager.inline.hpp#l32

I think current proposal is easy to understand if other people check
this with HotSpot.
Should I rename to "getByAddress" ?


Other your comment will be fixed in new webrev later.

Thanks,

Yasumasa


2017-09-29 14:35 GMT+09:00 serguei.spitsyn at oracle.com
<serguei.spitsyn at oracle.com>:
> Hi Yasumasa,
>
> Just some minor comments.
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/G1HeapRegionTable.java.frames.html
>
> I'd suggest to make the lines 144-145 a one-liner.
> It won't be that big. Otherwise, the indent is not right.
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegion.java.frames.html
>
>   The same as above for lines 85-86.
>   It seems, there is no reason for renaming 'type' to 't' in the
> initialize() method.
>
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegionManager.java.frames.html
>
>   89     public HeapRegion addrToRegion(Address addr) {
>   90       return regions().getByAddress(addr);
>   91     }
>
>   A suggestion: replace 'addrToRegion' with 'getByAddress'.
>   It will look similar to the 'heapRegionIterator.'
>
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/g1/HeapRegionType.java.html
>
>   41     private static int freeTag;
>   42
>   43     private static int youngMask;
>   44
>   45     private static int humongousMask;
>   46
>   47     private static int pinnedMask;
>   48
>   49     private static int oldMask;
>   50
>   51     private static CIntegerField tagField;
>
>   Unneeded empty lines.
>
>   Also, it looks like the fields 'freeTag' and 'pinnedMask' are never
> initialized.
>   Not sure, if it is intentional.
>
> Otherwise, the fix looks good to me.
>
> Thanks,
> Serguei
>
>
> On 9/28/17 18:20, Yasumasa Suenaga wrote:
>
> Hi Serguei,
>
> I added it to JBS:
>
> https://bugs.openjdk.java.net/browse/JDK-8187403?focusedCommentId=14119248&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14119248
>
> Sorry for my English. I'm not good at English...
>
>
> Please, don't worry about your English.
> Your description looks good.
> Thank you for the bug report update!
>
> Thanks,
> Serguei
>
>
> Yasumasa
>
>
>
> 2017-09-29 8:27 GMT+09:00 serguei.spitsyn at oracle.com
> <serguei.spitsyn at oracle.com>:
>
> Hi Yasumasa,
>
> Could you, please, also add some evaluation to the bug report about what is
> the root cause and how do you fix it?
>
> Thanks,
> Serguei
>
>
>
> On 9/26/17 18:10, Yasumasa Suenaga wrote:
>
> Hi all,
>
> I added noreg-hard label to JBS because this issue appears Stack
> Memory window on HSDB (GUI application). So it is hard to test.
>
>    https://bugs.openjdk.java.net/browse/JDK-8187403
>
>
> Thanks,
>
> Yasumasa
>
>
>
> 2017-09-26 23:55 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>:
>
> Hi all,
>
> I uploaded new webrev to be adapted to jdk10/hs:
>
>    http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.01/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2017/09/21 7:48, Yasumasa Suenaga wrote:
>
> PING:
>
> Have you checked this issue?
>
>     http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.00/
>
>
> Yasumasa
>
>
>
> On 2017/09/11 11:18, Yasumasa Suenaga wrote:
>
> Hi all,
>
> This review request is a part of [1].
>
>
> JBS:
>     https://bugs.openjdk.java.net/browse/JDK-8187403
>
> webrev:
>     http://cr.openjdk.java.net/~ysuenaga/JDK-8187403/webrev.00/
>
>
> I cannot access JPRT. So I need a sponsor.
>
>
> Thanks,
>
> Yasumasa
>
>
> [1]
>
> http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-September/021821.html
>
>