<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Kishor,<br>
<br>
<div class="moz-cite-prefix">On 10/31/2017 04:53 PM, Kharbas, Kishor
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:F89640DCD01A85489FCBA68183A6A0F3BCD4BDC5@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;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 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;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri",sans-serif;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:803305139;
mso-list-type:hybrid;
mso-list-template-ids:-515445420 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
{mso-level-start-at:2;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l1
{mso-list-id:940526086;
mso-list-type:hybrid;
mso-list-template-ids:-1397965002 123212038 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l1:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:.25in;
text-indent:-.25in;}
@list l1:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:.75in;
text-indent:-.25in;}
@list l1:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
margin-left:1.25in;
text-indent:-9.0pt;}
@list l1:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:1.75in;
text-indent:-.25in;}
@list l1:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:2.25in;
text-indent:-.25in;}
@list l1:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
margin-left:2.75in;
text-indent:-9.0pt;}
@list l1:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:3.25in;
text-indent:-.25in;}
@list l1:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
margin-left:3.75in;
text-indent:-.25in;}
@list l1:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
margin-left:4.25in;
text-indent:-9.0pt;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></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"><o:p> </o:p></p>
<p class="MsoNormal">Greetings,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I am continuing the earlier discussion [1]
with a revised issue number representing the implementation of
the JEP [2].<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I have addressed the remaining unaddressed
comments (copied below) in these webrevs -<o:p></o:p></p>
<p class="MsoNormal"><a
href="http://cr.openjdk.java.net/%7Ekkharbas/8190308/webrev.11/"
moz-do-not-send="true">http://cr.openjdk.java.net/~kkharbas/8190308/webrev.11/</a><o:p></o:p></p>
<p class="MsoNormal"><a
href="http://cr.openjdk.java.net/%7Ekkharbas/8190308/webrev.11_to_10/"
moz-do-not-send="true">http://cr.openjdk.java.net/~kkharbas/8190308/webrev.11_to_10/</a><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Also, I would appreciate a review of the
CSR for the new flag at
<a href="https://bugs.openjdk.java.net/browse/JDK-8190309"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8190309</a>.</p>
</div>
</blockquote>
CSR: Reviewed.<br>
<br>
<br>
<blockquote type="cite"
cite="mid:F89640DCD01A85489FCBA68183A6A0F3BCD4BDC5@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="MsoPlainText">> > - in that same thread there
has also been the question about the<o:p></o:p></p>
<p class="MsoPlainText">> > need <o:p></o:p></p>
<p class="MsoPlainText">> > for blocking the signals for
the process that has gone unanswered.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Removed the blocking of signals.<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">> > -
Arguments::finalize_vm_init_args: maybe at the place where
the <o:p></o:p></p>
<p class="MsoPlainText">> > change adds the warning/info
message about NUMA support being<o:p></o:p></p>
<p class="MsoPlainText">> > specific <o:p></o:p></p>
<p class="MsoPlainText">> > to the file system; also add
the same warning about UseLargePages<o:p></o:p></p>
<p class="MsoPlainText">> > that <o:p></o:p></p>
<p class="MsoPlainText">> > is located elsewhere.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText">> > Like "UseXXXX may not be
supported in some specific file system <o:p></o:p></p>
<p class="MsoPlainText">> > implementations." - or just
ignore as Andrew said in the other<o:p></o:p></p>
<p class="MsoPlainText">> > email thread.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText">> > Note that I am not sure that
the selected place is the correct<o:p></o:p></p>
<p class="MsoPlainText">> > place <o:p></o:p></p>
<p class="MsoPlainText">> > for such warning about
incompatible flags (and maybe <o:p></o:p></p>
<p class="MsoPlainText">> > UseNUMA/UseLargePages should
be forcibly disabled here? But then <o:p></o:p></p>
<p class="MsoPlainText">> > again, it does not hurt just
having it enabled?).<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText">> > I will let the runtime team
comment as a lot of that argument <o:p></o:p></p>
<p class="MsoPlainText">> > processing changed in jdk9.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText">> > Maybe
Arguments::check_vm_args_consistency() is better.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText">> > There is similar code in
Arguments::adjust_after_os()...<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">1. Moved the check from
finalize_vm_init_args() <span
style="font-size:9.5pt;font-family:Consolas">
to </span>check_vm_args_consistency()<o:p></o:p></p>
<p class="MsoPlainText">2. When UseNUMA flag is true, adaptive
logical group chunk resizing is used for Numa aware collectors
such as ParallelOld. Just like UseNUMA is disabled in
os::inti_2() in Linux when UseLargePages is set, it will be a
good idea to disable UseNUMA when the new option is used.<o:p></o:p></p>
<p class="MsoPlainText">3. I think its ok to leave
UseNUMAInterleaving ON as it can be used for other memory
areas.<o:p></o:p></p>
<p class="MsoPlainText">4. For the same reason I also do not
disable UseLargePages.<o:p></o:p></p>
<p class="MsoPlainText">5. About handling UseLargePages, I
thought of handling it the same way as when
reserve_memory_special() fails to allocate large pages, i.e.
generating a log_debug message.
<o:p></o:p></p>
<p class="MsoPlainText" style="margin-left:1.0in"> <i><span
style="font-family:"Courier New"">// failed; try
to reserve regular memory below</span></i><o:p></o:p></p>
<p class="MsoPlainText" style="margin-left:1.0in"><i><span
style="font-family:"Courier New""> if
(UseLargePages && (!FLAG_IS_DEFAULT(UseLargePages)
||</span></i><o:p></o:p></p>
<p class="MsoPlainText" style="margin-left:1.0in"><i><span
style="font-family:"Courier New"">
!FLAG_IS_DEFAULT(LargePageSizeInBytes))) {</span></i><o:p></o:p></p>
<p class="MsoPlainText" style="margin-left:1.0in"><i><span
style="font-family:"Courier New"">
log_debug(gc, heap, coops)("Reserve regular memory
without large pages");</span></i><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoPlainText">> > - here I may probably be
speaking wrongly on behalf of the<o:p></o:p></p>
<p class="MsoPlainText">> > runtime <o:p></o:p></p>
<p class="MsoPlainText">> > team, but os.hpp, as an
abstraction of the OS should probably not<o:p></o:p></p>
<p class="MsoPlainText">> > have <o:p></o:p></p>
<p class="MsoPlainText">> > platform specific ifdefs in
it.<o:p></o:p></p>
<p class="MsoPlainText">> > <o:p></o:p></p>
<p class="MsoNormal">and <o:p></o:p></p>
<p class="MsoPlainText">> > > - You removed
os::attempt_reserve_memory_at() from os.cpp and<o:p></o:p></p>
<p class="MsoPlainText">> > > split <o:p></o:p></p>
<p class="MsoPlainText">> > > into os_posix.cpp and
os_windows.cpp.<o:p></o:p></p>
<p class="MsoPlainText">> > > But I think you should
remain os::attempt_reserve_memory_at()<o:p></o:p></p>
<p class="MsoPlainText">> > > at <o:p></o:p></p>
<p class="MsoPlainText">> > > os.cpp and implement os
specific stuffs at each os_xxx.cpp files<o:p></o:p></p>
<p class="MsoPlainText">> > > for <o:p></o:p></p>
<p class="MsoPlainText">> > > pd_xxx. Of cource move
MemTracker function call as well.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoPlainText">In the updated webrev, I move the
implementation from os_posix.cpp and os_window.cpp to
respective pd_xxx functions. No AIX specific ifdef is required
now.</p>
</div>
</blockquote>
Thank you for all changes.<br>
<br>
I have minor nits now:<br>
==============================================<br>
- os***.cpp has some function names which include *dax*. I would
prefer not to include it. As you know, Thomas also pointed it.<br>
<br>
==============================================<br>
src/hotspot/share/runtime/arguments.cpp<br>
2537 if (!FLAG_IS_DEFAULT(UseNUMAInterleaving) ||
!FLAG_IS_DEFAULT(UseNUMA)) {<br>
- Don't we need to check these 2 flags' value to be true? i.e. if
user sets to false, below message will be printed.<br>
<br>
==============================================<br>
test/hotspot/jtreg/gc/TestAllocateHeapAt.java<br>
- On other discussion, I mentioned to test only for Windows and
Linux as the JEP described only about those 2. But without *dax*
function names, it seems like not filtering OS seems okay too.<br>
<br>
2 * Copyright (c) 2017, xxx Oracle and/or its affiliates. All
rights reserved.<br>
- Please remove 'xxx '.<br>
<br>
47 Collections.addAll(vmOpts, new String[]
{"-XX:AllocateHeapAt="+test_dir,<br>
- Add spaces. '+test_dir' -> ' + test_dir'<br>
<br>
49 "-Xmx50m",<br>
50 "-Xms50m",<br>
- You said there were no special reason for 200m(heap size of
webrev.10) on other discussion. I would recommend 32m.<br>
<br>
FYI, I ran hs-tier1 and hs-tier2 with your patch and the result is
good. i.e. no new failures.<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<blockquote type="cite"
cite="mid:F89640DCD01A85489FCBA68183A6A0F3BCD4BDC5@ORSMSX116.amr.corp.intel.com">
<div class="WordSection1">
<p class="MsoPlainText"><o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">Thank you and looking forward for your
feedback.<o:p></o:p></p>
<p class="MsoPlainText">- Kishor<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoNormal">[1] <a
href="http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2017-October/020682.html"
moz-do-not-send="true">
http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2017-October/020682.html</a><o:p></o:p></p>
<p class="MsoNormal">[2] <a
href="https://bugs.openjdk.java.net/browse/JDK-8171181"
moz-do-not-send="true">
https://bugs.openjdk.java.net/browse/JDK-8171181</a><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
<br>
</body>
</html>