<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Hi Derek,<br>
<br>
Sorry for taking so long to review this.<br>
<br>
<div class="moz-cite-prefix">On 2014-11-18 17:40, Derek White wrote:<br>
</div>
<blockquote cite="mid:546B7683.9000803@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
Hi Team,<br>
<br>
First review request. Please let me know if I've forgotten
something or have gone completely off the rails.<br>
<br>
The main point of this bug is to remove deprecated -XX options
which are alias for other options.<br>
<br>
The only complicated part is that one case,
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<i>CMSParPromoteBlocksToClaim</i> was not a true alias for <i>OldPLABSize</i>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
but a parallel option with different defaults that were
synchronized in ergo processing. This fix removes the <i>CMSParPromoteBlocksToClaim
</i>variable but preserves using different defaults in the CMS
case.<br>
<br>
Also in this fix I added warning messages to several remaining
undocumented command line options aliases. This should ease
removal of these options in the future<br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<pre>CMSMarkStackSize ==> MarkStackSize (MarkStackSize not documented either, but came in jdk6)
G1MarkStackSize ==> MarkStackSize
CMSMarkStackSizeMax ==> MarkStackSizeMax (MarkStackSizeMax not documented either)
ParallelMarkingThreads => ConcGCThreads (ConcGCThreads came in jdk6)
ParallelCMSThreads ==> ConcGCThread<span class="new"></span></pre>
<br>
Thanks,<br>
- Derek<br>
<br>
<b>Webrev</b>: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8061611/webrev.00/">http://cr.openjdk.java.net/~drwhite/8061611/webrev.00/</a><br>
<br>
<b>Bug</b>: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8061611">https://bugs.openjdk.java.net/browse/JDK-8061611</a><br>
</blockquote>
<br>
Looks good to me. One minor nit regarding the naming of the new
constants that you introduce:<br>
<br>
682 static const int defaultDynamicOldPLABSize = 16;<br>
683 static const int defaultStaticOldPLABSize = 50;<br>
<br>
HotSpot does not have a common guideline for how to name constants.
There are at least three widely used naming standards:<br>
<br>
- All caps: DEFAULT_DYNAMIC_OLD_PLAB_SIZE<br>
- Like an instance variable: _default_dynamic_old_plab_size<br>
- Like a flag: DefaultDynamicOldPLABSize<br>
<br>
I think it is hard to say which one is "standard" but I would prefer
if we don't introduce yet another naming standard for constants. Do
you mind changing to one of these? Personally I prefer the last one
(the "Like a flag" version).<br>
<br>
Bengt<br>
<br>
<blockquote cite="mid:546B7683.9000803@oracle.com" type="cite"> <br>
<b>Testing</b>:<br>
jprt:<br>
Saw 1-2 intermittent failures that went away on retesting -
hangs and timeouts.<br>
<br>
refworkload:<br>
no differences<br>
<br>
jtreg: Saw a few unexplained results. Not sure if typical or not:<br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<blockquote>
<h4>Execution failed: `main' threw exception:
java.lang.Exception: jmap -heap exited with error code: 1</h4>
<ul>
<li> <a moz-do-not-send="true"
href="http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.jtr">gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java</a>:
Checks that jmap -heap contains the flag
CompressedClassSpaceSize </li>
</ul>
<h4>Program
`/export/users/drwhite/test-builds/j2sdk-image.11.17.2014/bin/java'
interrupted! (timed out?)</h4>
<ul>
<li> <a moz-do-not-send="true"
href="http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/closed/runtime/AppCDS/SharedArchiveConsistency.jtr">closed/runtime/AppCDS/SharedArchiveConsistency.java</a>:
SharedArchiveConsistency </li>
</ul>
Plus a bunch of tests that are labelled "ignored".<br>
</blockquote>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<p><br>
</p>
</blockquote>
<br>
</body>
</html>