<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Kishor,<br>
<br>
<div class="moz-cite-prefix">On 11/21/18 11:17 PM, Kharbas, Kishor
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:F89640DCD01A85489FCBA68183A6A0F3BEC69984@ORSMSX116.amr.corp.intel.com">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNormal">Hi all,<o:p></o:p></p>
<p class="MsoNormal">Requesting review of the patch for
allocating old generation of parallelold gc on alternate
memory devices such nv-dimm.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The design of this implementation is
explained on the bug page -
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8211424">https://bugs.openjdk.java.net/browse/JDK-8211424</a>.<o:p></o:p></p>
<p class="MsoNormal">This is subtask of <a
href="https://bugs.openjdk.java.net/browse/JDK-8202286"
moz-do-not-send="true">
https://bugs.openjdk.java.net/browse/JDK-8202286</a> which
is implementation for parallel old gc.<o:p></o:p></p>
<p class="MsoNormal">Please follow the parent issue here : <a
href="https://bugs.openjdk.java.net/browse/JDK-8202286"
moz-do-not-send="true">
https://bugs.openjdk.java.net/browse/JDK-8202286</a>.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">(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.’)<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Webrev: <a
href="http://cr.openjdk.java.net/%7Ekkharbas/8211424/webrev.04"
moz-do-not-send="true">
http://cr.openjdk.java.net/~kkharbas/8211424/webrev.04</a><o:p></o:p></p>
<p class="MsoNormal">
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kkharbas/8211424/webrev.03_to_04">http://cr.openjdk.java.net/~kkharbas/8211424/webrev.03_to_04</a><o:p></o:p></p>
</div>
</blockquote>
Webrev.3 looks okay in general, just one minor nit.<br>
<br>
You added is_hetero_heap() at ParallelScavengeHeap class.<br>
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<br>
2000 if (!(UseAdaptiveSizePolicy && UseAdaptiveGCBoundary)
||<br>
2001 ParallelScavengeHeap::heap()->is_hetero_heap()) {<br>
2002 return false;<br>
2003 }<br>
<br>
2. If you still prefer to have the method/'member variable':<br>
a) 154 bool is_hetero_heap() { return _is_hetero_heap; }<br>
- Add 'const'<br>
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.<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<blockquote type="cite"
cite="mid:F89640DCD01A85489FCBA68183A6A0F3BEC69984@ORSMSX116.amr.corp.intel.com">
<div class="WordSection1">
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks<o:p></o:p></p>
<p class="MsoNormal">Kishor<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
<br>
</body>
</html>