<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>Thanks Gerard,<br><br></div><div><br>On Jan 20, 2015, at 6:59 PM, Gerard Ziemski <<a href="mailto:gerard.ziemski@oracle.com">gerard.ziemski@oracle.com</a>> wrote:<br><br></div><blockquote type="cite"><div>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<pre wrap="">hi Derek ,
Very, very nice code and thank you for taking the time to do a bit of code cleanup. I have just a few comments/suggestions below for the “arguments.cpp<a class="moz-txt-link-rfc2396E" href="file:#1Insteadof{">" file:
#1 Instead of
{ "</a>MaxGCMinorPauseMillis", JDK_Version::jdk(8), JDK_Version::jdk(SpecialFlag::_removal_unscheduled)},
how about this:
{ "MaxGCMinorPauseMillis", JDK_Version::jdk(8), JDK_Version::jdk(JDK_Version::unscheduled())},
and move "bool is_removal_scheduled() const” API to JDK_Version class? It seems to me that any API that has to do with JDK version number should live in JDK_Version class.
</pre></div></blockquote><div><br></div>How about "JDK_Version::undefined()" instead? That seems more generally useful and less option-specific<br><blockquote type="cite"><div><pre wrap="">
#2 Instead of
.
.
.
{ "CMSMarkStackSizeMax", JDK_Version::jdk(9), JDK_Version::jdk(10)},
{ "CMSMarkStackSize", JDK_Version::jdk(9), JDK_Version::jdk(10)},
.
.
.
how about going back to the nice formatting we used to have like:
.
.
.
{ "CMSMarkStackSizeMax”, JDK_Version::jdk(9), JDK_Version::jdk(10)},
{ "CMSMarkStackSize”, JDK_Version::jdk(9), JDK_Version::jdk(10)},
.
.
.
The formatted text seems so much nicer to read.
</pre></div></blockquote><div><br></div>OK. I thought I did that. Must have gotten lost.<br><blockquote type="cite"><div><pre wrap="">#3 Where did the JDK version numbers in deprecated_jvm_flags table come from? Are we sure we got them right?
</pre></div></blockquote>I'll double check those. <br><blockquote type="cite"><div><pre wrap="">
cheers</pre></div></blockquote><div><br></div>Thanks again.<div><br></div><div>- Derek</div><div><br></div><div><br><blockquote type="cite"><div>
<div class="moz-cite-prefix">On 1/19/2015 12:56 PM, Derek White
wrote:<br>
</div>
<blockquote cite="mid:54BD5378.3010500@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Gerard found a few portability bugs.
3rd request's the charm.<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Edrwhite/8066821/webrev.02/">http://cr.openjdk.java.net/~drwhite/8066821/webrev.02</a><br>
<br>
Reran jprt and jtreg.<br>
<br>
- Derek<br>
<br>
On 1/12/15 5:10 PM, Derek White wrote:<br>
</div>
<blockquote cite="mid:54B44661.8070007@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Edrwhite/8066821/webrev.00/">http://cr.openjdk.java.net/~drwhite/8066821/webrev.00/</a><br>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8066821">https://bugs.openjdk.java.net/browse/JDK-8066821</a><br>
<br>
This webrev adds support for handling "<i>deprecated</i>" -XX
options (options that still <b>do</b> something but are planned
for removal) and "<i>alias</i>" options (alternate names for
other -XX options) by simply adding entries to the
deprecated_jvm_flags and/or aliased_jvm_flags tables. This
follows the<br>
example of the existing obsolete_jvm_flags table.<br>
<br>
This replaces a lot of ad-hoc and occasionally wrong code in
arguments.cpp (including Arguments::check_deprecated_gc_flags)
as well as supporting automatically disabling options after a
certain version.<br>
<br>
Removed Code:<br>
- Removed global DefaultMaxRAMFraction, which was an "improper"
alias for "MaxRAMFraction" (two variables that were roughly kept
in sync vs. two names for the same variable).<br>
- Arguments::check_deprecated_gc_flags().<br>
- Alias handling code in Arguments::parse_each_vm_init_arg().<br>
It also avoids future ad-hoc and occasionally wrong code as new
options get aliased and deprecated.<br>
<br>
Tests:<br>
Deprecated and alias options can be tested by adding entries to
tables in new tests:<br>
VMAliasOptions.java<br>
VMDeprecatedOptions.java<br>
<br>
The new tests subsume these existing tests:<br>
test/gc/startup_warnings/TestDefaultMaxRAMFraction.java<br>
test/gc/startup_warnings/TestNoParNew.java <br>
<br>
<br>
Tests run:<br>
jprt<br>
jtreg<br>
<br>
Thanks,<br>
<br>
- Derek<br>
</blockquote>
<br>
</blockquote>
<br>
</div></blockquote></div></body></html>