[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-gc-dev
Subject: Re: RFR (smallish?) 8061611: Remove deprecated command line flags
From: Jon Masamitsu <jon.masamitsu () oracle ! com>
Date: 2014-11-24 18:21:18
Message-ID: 5473771E.8000202 () oracle ! com
[Download RAW message or body]
On 11/24/2014 03:55 AM, Bengt Rutisson wrote:
>
> Hi Derek,
>
> Sorry for taking so long to review this.
>
> On 2014-11-18 17:40, Derek White wrote:
> > Hi Team,
> >
> > First review request. Please let me know if I've forgotten something
> > or have gone completely off the rails.
> >
> > The main point of this bug is to remove deprecated -XX options which
> > are alias for other options.
> >
> > The only complicated part is that one case,
> > /CMSParPromoteBlocksToClaim/ was not a true alias for /OldPLABSize/
> > but a parallel option with different defaults that were synchronized
> > in ergo processing. This fix removes the /CMSParPromoteBlocksToClaim
> > /variable but preserves using different defaults in the CMS case.
> >
> > 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
> > 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
> >
> > Thanks,
> > - Derek
> >
> > *Webrev*: http://cr.openjdk.java.net/~drwhite/8061611/webrev.00/
> >
> > *Bug*: https://bugs.openjdk.java.net/browse/JDK-8061611
>
> Looks good to me. One minor nit regarding the naming of the new
> constants that you introduce:
>
> 682 static const int defaultDynamicOldPLABSize = 16;
> 683 static const int defaultStaticOldPLABSize = 50;
>
> HotSpot does not have a common guideline for how to name constants.
> There are at least three widely used naming standards:
>
> - All caps: DEFAULT_DYNAMIC_OLD_PLAB_SIZE
> - Like an instance variable: _default_dynamic_old_plab_size
> - Like a flag: DefaultDynamicOldPLABSize
>
> 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).
Sorry for this Derek, but I prefer the "_default_dynamic_old_plab_size"
because of the leading "_". It is least likely to confuse me (as does
like a flag or like an All caps macro).
But as Bengt says, there is no standard.
Jon
>
> Bengt
>
> >
> > *Testing*:
> > jprt:
> > Saw 1-2 intermittent failures that went away on retesting - hangs
> > and timeouts.
> >
> > refworkload:
> > no differences
> >
> > jtreg: Saw a few unexplained results. Not sure if typical or not:
> >
> >
> > Execution failed: `main' threw exception:
> > java.lang.Exception: jmap -heap exited with error code: 1
> >
> > * gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java
> > <http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.jtr>:
> > Checks that jmap -heap contains the flag
> > CompressedClassSpaceSize
> >
> >
> > Program
> > `/export/users/drwhite/test-builds/j2sdk-image.11.17.2014/bin/java'
> > interrupted! (timed out?)
> >
> > * closed/runtime/AppCDS/SharedArchiveConsistency.java
> > <http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/closed/runtime/AppCDS/SharedArchiveConsistency.jtr>:
> > SharedArchiveConsistency
> >
> > Plus a bunch of tests that are labelled "ignored".
> >
> >
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 11/24/2014 03:55 AM, Bengt Rutisson
wrote:<br>
</div>
<blockquote cite="mid:54731CA0.4030302@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<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>
</blockquote>
<br>
Sorry for this Derek, but I prefer the
"_default_dynamic_old_plab_size"<br>
because of the leading "_". It is least likely to confuse me (as
does<br>
like a flag or like an All caps macro).<br>
<br>
But as Bengt says, there is no standard.<br>
<br>
Jon<br>
<br>
<blockquote cite="mid:54731CA0.4030302@oracle.com" type="cite"> <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/h \
otspot/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/h \
otspot/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>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic