RFR(M): 8211424: Allocation of old generation of java heap on alternate memory devices - ParallelOld

[sangheon.kim at oracle.com]

Hi Kishor,

On 11/21/18 11:17 PM, Kharbas, Kishor wrote:
>
> Hi all,
>
> Requesting review of the patch for allocating old generation of 
> parallelold gc on alternate memory devices such nv-dimm.
>
> The design of this implementation is explained on the bug page - 
> https://bugs.openjdk.java.net/browse/JDK-8211424.
>
> This is subtask of https://bugs.openjdk.java.net/browse/JDK-8202286 
> <https://bugs.openjdk.java.net/browse/JDK-8202286> which is 
> implementation for parallel old gc.
>
> Please follow the parent issue here : 
> https://bugs.openjdk.java.net/browse/JDK-8202286 
> <https://bugs.openjdk.java.net/browse/JDK-8202286>.
>
> (PS: this is continuation of old thread 'RFR(M): 8204908: NVDIMM for 
> POGC and G1GC - ReserveSpace.cpp changes are mostly eliminated/no 
> collector specific code.’)
>
> Webrev: http://cr.openjdk.java.net/~kkharbas/8211424/webrev.04 
> <http://cr.openjdk.java.net/%7Ekkharbas/8211424/webrev.04>
>
> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.03_to_04
>
Webrev.3 looks okay in general, just one minor nit.

You added is_hetero_heap() at ParallelScavengeHeap class.
1. Is this method/'member variable' really needed? Just checking 
'AllocateOldGenAt != NULL && UseAdaptiveGCBoundary' seems enough. And 
this looks similar condition check as the line 2000 at psParallelCompact.cpp
2000   if (!(UseAdaptiveSizePolicy && UseAdaptiveGCBoundary) ||
2001       ParallelScavengeHeap::heap()->is_hetero_heap()) {
2002     return false;
2003   }

2. If you still prefer to have the method/'member variable':
a)  154   bool is_hetero_heap() { return _is_hetero_heap; }
      - Add 'const'
b) The name seems misleading because _is_hetero_heap is enabled when 
both 'AllocateOldGenAt' and 'UseAdaptiveGCBoundary' are set. i.e. if 
'AllocateOldGenAt' is only set, _is_hetero_heap will be false but still 
we are using nvdimm for old gen, just not use 
AdjoiningGenerationsForHeteroHeap class. So any meaning of adaptive gc 
boundary should be added on the name.

Thanks,
Sangheon


> Thanks
>
> Kishor
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181127/ec3fb591/attachment.htm>