<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Please see inline.<br>
<br>
Tao<br>
<br>
On 10/8/13 2:03 PM, Jon Masamitsu wrote:
<blockquote cite="mid:52547332.10409@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<pre> product(double, CMSSmallSplitSurplusPercent, 1.10, \
<span class="removed">- "CMS: the factor by which to inflate estimated demand of small" \</span>
<span class="removed">- " block sizes to prevent splitting to supply demand for smaller" \</span>
<span class="removed">- " blocks") \</span>
<span class="new">+ "CMS: the factor by which to inflate estimated demand of small " \</span>
<span class="new">+ "block sizes to prevent splitting to supply demand for smaller" \</span>
<span class="new">+ "blocks")
Second line of next text, space after "smaller".</span></pre>
</blockquote>
will do.<br>
<blockquote cite="mid:52547332.10409@oracle.com" type="cite">
<pre><span class="new">
</span> product(uintx, CMS_SweepWeight, 75, \
<span class="removed">- "Percentage (0-100) used to weight the current sample when " \</span>
<span class="new">+ "Percentage (0-100) used to weigh the current sample when "
I think the original is correct.
</span></pre>
weight<br>
<span class="main-fl"><em>transitive verb</em></span>
<div class="ld_on_collegiate">
<p class="bottom_entry">: to put a weight on (something) to make
it heavier or to keep it from moving</p>
</div>
<div class="sblk">
<div class="snum">4</div>
<div class="scnt"><span class="ssens"> <strong>:</strong> to
assign a statistical weight to<br>
<br>
<br>
What do you think?<br>
<br>
</span></div>
</div>
</blockquote>
Umm...you are right.<br>
<blockquote cite="mid:52547332.10409@oracle.com" type="cite">
<div class="sblk">
<div class="scnt"><span class="ssens"> </span><br>
<pre> manageable(intx, CMSAbortablePrecleanWaitMillis, 100, \
<span class="removed">- "(Temporary, subject to experimentation)" \</span>
<span class="removed">- " Time that we sleep between iterations when not given" \</span>
<span class="removed">- " enough work per iteration") \</span>
<span class="new">+ "(Temporary, subject to experimentation) " \</span>
<span class="new">+ "Time that we sleep between iterations when not given" \</span>
<span class="new">+ "enough work per iteration")
Second line of new text, space after "given"</span></pre>
</div>
</div>
</blockquote>
will do.<br>
<blockquote cite="mid:52547332.10409@oracle.com" type="cite">
<div class="sblk">
<div class="scnt">
<pre><span class="new">
</span> notproduct(bool, PrintFlagsWithComments, false, \
<span class="removed">- "Print all VM flags with default values and descriptions and exit")\</span>
<span class="new">+ "Print all VM flags with default values and descriptions and exit") \
Not sure on this. Sometimes I don't see the right number of
blanks but do you want a separate line so "\" line up?
</span></pre>
</div>
</div>
</blockquote>
Okay, doable.<br>
<blockquote cite="mid:52547332.10409@oracle.com" type="cite">
<div class="sblk">
<div class="scnt">
<pre><span class="new">
Thanks all.
Jon
</span></pre>
<span class="ssens"><br>
</span></div>
</div>
<div class="moz-cite-prefix">On 10/4/2013 5:08 PM, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:524F5873.8050309@oracle.com" type="cite">Hi
all, <br>
<br>
A new webrev responding to Jon's recent review <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.05/">http://cr.openjdk.java.net/~tamao/8010506/webrev.05/</a>
<br>
<br>
Thanks. <br>
Tao <br>
<br>
On 10/4/13 4:51 PM, Tao Mao wrote: <br>
<blockquote type="cite">Forwarded Jon's review comments to the
main thread. <br>
<br>
Tao <br>
<br>
On 10/4/13 2:53 PM, Jon Masamitsu wrote: <br>
<blockquote type="cite">If my comments conflict with any other
comments, ignore <br>
might. <br>
<br>
product(bool, MaxFDLimit,
true, \ <br>
- "Bump the number of file descriptors to max in
solaris.") \ <br>
+ "Bump the number of file descriptors to max in
solaris") \ <br>
<br>
I would prefer "maximum" instead of "max". There are
examples of both in the <br>
file. If we're standarizing on spelling, I prefer t he
complete word. <br>
<br>
<br>
492 product(bool, JavaMonitorsInStackTrace,
true, \ <br>
493 "Print info about Java monitor locks when the
stacks are dumped") \ <br>
<br>
Spell out "info". Should be "information". <br>
<br>
<br>
product(bool, UseSerialGC,
false, \ <br>
- "Use the serial garbage
collector") \ <br>
+ "Use the Serial garbage collector") <br>
<br>
I think "serial" is more correct that "Serial". <br>
<br>
<br>
<br>
<br>
I would have changed this <br>
<br>
584 "Shows oopmap generation") <br>
<br>
to <br>
<br>
<br>
584 "Shows oop map generation") <br>
<br>
I don't know what the phase "Show OopMapGeneration" means. <br>
<br>
+ "Show message about safepoint synch") <br>
Change "synch" to "synchronization" \
<br>
<br>
<br>
+ "directory) of the dump file (defaults to
java_pid<pid>.hprof " \ <br>
<br>
Why the extra space at the end of " ...
java_pid<pid>.hprof " <br>
<br>
<br>
<br>
<br>
<br>
+ "Maximum compaction in the Parallel Old garbage
collector for " \ <br>
"a system GC") <br>
<br>
<br>
"Maximum compaction" -> "Use maximum compaction" <br>
<br>
<br>
<br>
<br>
+ "Always tenure objects in eden. (Parallel GC
only)") \ <br>
<br>
"Parallel GC" does not start a sentence so I would use <br>
"parallel GC". <br>
<br>
<br>
<br>
+ "Use adaptive free Lists in the CMS
generation") \ <br>
<br>
"Lists" -> "lists" <br>
<br>
- "Verbose output for parallel
GC.") \ <br>
+ "Verbose output for parallel
gc") \ <br>
<br>
I've seen "parallel GC" before in the changes. I <br>
prefer "parallel GC" to "parallel gc" <br>
<br>
<br>
+ "it is disabled because this currently conflicts
with " \ <br>
+ "parallel card scanning under certain
conditions.") \ <br>
<br>
Did you intend the period after "conditions"? <br>
<br>
<br>
+ "Minimum percentage (0-100) of the CMS
incremental duty cycle " \ <br>
+ "when CMSIncrementalPacing is enable") <br>
<br>
prefer "used whenCMSIncrementalPacing is
enable" \ <br>
<br>
<br>
<br>
<br>
+ "Use Mark-Sweep-Compact algorithm at full
collections") <br>
<br>
Prefer "mark-sweek-compact" to "Mark-Sweep-Compact" <br>
<br>
<br>
<br>
<br>
+ "Whether we should simulate frequent marking
stack / work queue " \ <br>
+ "overflow") <br>
<br>
<br>
"Whether we should simulate" -> "Simulate" <br>
More consistent with other changes. <br>
<br>
<br>
<br>
notproduct(uintx, CMSMarkStackOverflowInterval,
1000, \ <br>
- "An `interval' counter that determines how
frequently" \ <br>
- " we simulate overflow; a smaller number
increases frequency") \ <br>
+ "An \"interval\" counter that determines how
frequently " \ <br>
+ "we simulate overflow; a smaller number increases
frequency") <br>
<br>
"we simulate overflow" -> "to simulate overflow" <br>
<br>
<br>
product(bool, CMSConcurrentMTEnabled,
true, \ <br>
- "Whether multi-threaded concurrent work enabled
(if ParNewGC)") \ <br>
+ "Whether multi-threaded concurrent work enabled
" \ <br>
+ "(effective only if ParNewGC)") <br>
<br>
"Whether multi-threaded concurrent work enabled" <br>
-> <br>
"Enablemulti-threaded concurrent work " <br>
<br>
<br>
<br>
product(uintx, CMSPrecleanThreshold,
1000, \ <br>
- "Don't re-iterate if #dirty cards less than
this") \ <br>
+ "Do not re-iterate if number of dirty cards is
less than this") \ <br>
<br>
<br>
"Do not re-iterate ..." -> "Do not iterate again ..." <br>
<br>
<br>
product(bool, UseAdaptiveSizePolicyWithSystemGC,
false, \ <br>
- "Use statistics from System.GC for adaptive size
policy") \ <br>
+ "Use statistics from System.gc() for adaptive
size policy") <br>
<br>
"Use statistics" -> "Include statistics" <br>
<br>
<br>
diagnostic(bool, VerifySilently,
false, \ <br>
- "Don't print print the verification
progress") \ <br>
+ "Do not print print the verification
progress") \ <br>
<br>
"print print" -> "print" <br>
<br>
<br>
product(bool, DisableExplicitGC,
false, \ <br>
- "Tells whether calling System.gc() does a full
GC") \ <br>
+ "Tell whether calling System.gc() does a full
GC") \ <br>
<br>
"Tell whether calling" -> "Disable (ignore) calls to" <br>
<br>
<br>
<br>
- "Print the time taken by each parallel old gc
phase." \ <br>
- "PrintGCDetails must also be
enabled.") \ <br>
+ "Print the time taken by each ParallelOldGC phase
" \ <br>
+ "(PrintGCDetails must also be enabled)") <br>
<br>
I prefer the original with "gc" -> "GC" <br>
<br>
or <br>
<br>
"Print the time taken by each ParallelOldGC phase" <br>
-> <br>
"Print the time taken by each phase with UseParallelOldGC" <br>
<br>
<br>
develop(bool, TraceParallelOldGCMarkingPhase,
false, \ <br>
- "Trace parallel old gc marking
phase") \ <br>
+ "Trace ParallelOldGC marking phase") <br>
<br>
Prefer original with "gc" -> "GC" <br>
<br>
or <br>
<br>
"Trace parallel old gc marking phase" <br>
<br>
-> <br>
"Trace marking phases with UseParallelOldGC" <br>
<br>
<br>
Similarly with <br>
<br>
develop(bool, TraceParallelOldGCSummaryPhase,
false, \ <br>
- "Trace parallel old gc summary
phase") \ <br>
+ "Trace ParallelOldGC summary
phase") \ <br>
\ <br>
develop(bool, TraceParallelOldGCCompactionPhase,
false, \ <br>
- "Trace parallel old gc compaction
phase") \ <br>
+ "Trace ParallelOldGC compaction
phase") \ <br>
\ <br>
develop(bool, TraceParallelOldGCDensePrefix,
false, \ <br>
- "Trace parallel old gc dense prefix
computation") \ <br>
+ "Trace ParallelOldGC dense prefix
computation") \ <br>
<br>
<br>
<br>
<br>
+ "Rotatingly use gclog files (for long running
applications). " \ <br>
+ "It requires -Xloggc:<filename>") <br>
<br>
<br>
"Rotatingly use gclog" -> "Rotate gclog" <br>
<br>
<br>
<br>
product(bool, UseCompiler,
true, \ <br>
- "use
compilation")
\ <br>
+ "Use compilation") <br>
<br>
"Use compilation" -> "Use Just-In-Time compilation" <br>
<br>
<br>
<br>
- "how many entries we'll try to leave on the stack
during " \ <br>
- "parallel
GC") \ <br>
+ "Number of entries we will try to leave on the
stack " \ <br>
+ "during parallel gc" <br>
<br>
I prefer "GC" to "gc". <br>
<br>
I will probably not have time to respond to any reply <br>
you have until next week. <br>
<br>
<br>
Jon </blockquote>
<br>
<br>
On 10/3/13 7:58 PM, Jesper Wilhelmsson wrote: <br>
<blockquote type="cite">Looks good! <br>
Thanks for doing this! <br>
/Jesper <br>
<br>
Tao Mao skrev 3/10/13 11:27 PM: <br>
<blockquote type="cite">Hi all, <br>
<br>
Mikael V. and Jesper's points are taken. There's turned
out to be a bunch of <br>
changes. <br>
<br>
Here's the new webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.04/">http://cr.openjdk.java.net/~tamao/8010506/webrev.04/</a>
<br>
<br>
CR: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8010506">https://bugs.openjdk.java.net/browse/JDK-8010506</a>
<br>
<br>
Please review it so that I can get it in before ZBB. <br>
<br>
Thanks. <br>
Tao <br>
<br>
On 10/2/13 4:54 PM, Jesper Wilhelmsson wrote: <br>
<blockquote type="cite">Hi Tao, <br>
<br>
I noticed that in a few places there is a space between
the description string <br>
and the right parenthesis. E.g. <br>
<br>
product(intx, SyncVerbose, 0, "(Unstable)" ) <br>
<br>
Search for '" )' and you will find a few. <br>
<br>
There is also a capital S in "LAB'S" in the description
of <br>
CMSParPromoteBlocksToClaim. <br>
<br>
<br>
I didn't mention these before since I really want this
to be pushed asap, but <br>
if you decide to act on Mikael's comments about Print vs
Prints maybe you can <br>
fix these minor things as well? <br>
<br>
I don't think a new webrev is needed for these changes.
<br>
<br>
Thanks, <br>
/Jesper <br>
<br>
<br>
Mikael Vidstedt skrev 3/10/13 1:40 AM: <br>
<blockquote type="cite"> <br>
Tao, <br>
<br>
I sampled the changes and this is definitely a great
clean up! <br>
<br>
Question: Do you want to attack inconsistencies like
"Print" vs "Prints" etc. <br>
since you're changing so much of this anyway? <br>
<br>
For example I see that you changed this: <br>
<br>
product(bool, ProfileIntervals,
false, \ <br>
- "Prints profiles for each interval (see
ProfileIntervalsTicks)") \ <br>
+ "Print profiles for each interval (see
ProfileIntervalsTicks)") \ <br>
<br>
<br>
But there's still this: <br>
<br>
develop(bool, TraceClearedExceptions,
false, \ <br>
"Prints when an exception is forcibly
cleared") \ <br>
<br>
<br>
and this: <br>
<br>
3237 develop(intx, TraceBytecodesAt, <br>
0, \ <br>
3238 "Traces bytecodes starting with
specified bytecode <br>
number") \ <br>
<br>
3596 develop(bool, PerfTraceDataCreation, <br>
false, \ <br>
3597 "Trace creation of Performance Data <br>
Entries") \ <br>
<br>
Cheers, <br>
Mikael <br>
<br>
On 2013-10-01 18:00, Tao Mao wrote: <br>
<blockquote type="cite">Hi all, <br>
<br>
Jesper's suggestion is integrated. <br>
<br>
new webrev: "Frame" seems to be the best way to
review it. <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.03/">http://cr.openjdk.java.net/~tamao/8010506/webrev.03/</a>
<br>
<br>
Please review it so that I can get it in before ZBB.
<br>
<br>
Thanks. <br>
Tao <br>
<br>
On 9/24/13 12:47 PM, Jesper Wilhelmsson wrote: <br>
<blockquote type="cite">Hi Tao, <br>
<br>
The change looks good. Thanks for fixing this! <br>
<br>
Purely esthetic's: <br>
Lines 1820, 1821, 2221 and 2308 in your new
globals.hpp needs their \ aligned. <br>
<br>
Since you are touching pretty much the whole file
there are an inconsistency <br>
that would be nice to get rid of once and for all.
The following flags have <br>
their descriptions end with a period although they
are only one sentence. <br>
Most descriptions do not have the period at the
end. <br>
<br>
TracePageSizes <br>
InlineMathNatives <br>
ShowSafepointMsgs <br>
MaxFDLimit <br>
PredictedLoadedClassCount <br>
ParallelOldDeadWoodLimiterMean <br>
ParallelOldDeadWoodLimiterStdDev <br>
ScavengeWithObjectsInToSpace <br>
UseParNewGC <br>
ParallelGCVerbose <br>
ParallelGCBufferWastePct <br>
ParallelGCRetainPLAB <br>
PLABWeight <br>
CMSParPromoteBlocksToClaim <br>
OldPLABWeight <br>
AlwaysPreTouch <br>
CMSExpAvgFactor <br>
CMS_FLSWeight <br>
CMS_FLSPadding <br>
CMS_SweepPadding <br>
CMSDumpAtPromotionFailure <br>
CMSPrintChunksInDump <br>
CMSPrintObjectsInDump <br>
CMSVerifyReturnedBytes <br>
ExecuteInternalVMTests <br>
ConcGCYieldTimeout <br>
PrintParallelOldGCPhaseTimes <br>
ObjectCountCutOffPercent <br>
AbortVMOnExceptionMessage <br>
MaxBCEAEstimateLevel <br>
MaxBCEAEstimateSize <br>
CodeCacheMinimumFreeSpace <br>
CodeCacheMinBlockLength <br>
ExitOnFullCodeCache <br>
Tier0InvokeNotifyFreqLog <br>
Tier2InvokeNotifyFreqLog <br>
Tier3InvokeNotifyFreqLog <br>
Tier0BackedgeNotifyFreqLog <br>
Tier2BackedgeNotifyFreqLog <br>
Tier3BackedgeNotifyFreqLog <br>
Tier3CompileThreshold <br>
Tier4CompileThreshold <br>
IncreaseFirstTierCompileThresholdAt <br>
UsePerfData <br>
PerfDataMemorySize <br>
DumpSharedSpaces <br>
<br>
<br>
Thanks! <br>
/Jesper <br>
<br>
<br>
Tao Mao skrev 27/6/13 7:05 PM: <br>
<blockquote type="cite">new webrev: "Frame" seems
to be the best way to review it. <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.02/">http://cr.openjdk.java.net/~tamao/8010506/webrev.02/</a>
<br>
<br>
Suggestion are taken mostly from Thomas. <br>
<br>
Thanks. <br>
Tao <br>
<br>
On 5/14/13 2:46 PM, Jon Masamitsu wrote: <br>
<blockquote type="cite"> <br>
On 4/30/13 4:58 PM, Tao Mao wrote: <br>
<blockquote type="cite">new webrev <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.01/">http://cr.openjdk.java.net/~tamao/8010506/webrev.01/</a>
<br>
<br>
It needs reviews. <br>
</blockquote>
<br>
Tao, <br>
<br>
Changes you made look good. I did not see a
list of the <br>
suggested changes in the CR <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-8010506">https://jbs.oracle.com/bugs/browse/JDK-8010506</a>
<br>
<br>
so don't have any comment about what you
decided not <br>
to change. <br>
<br>
<blockquote type="cite"> <br>
Changeset: <br>
Thank you for the thorough report of all
these typos/errros, Thomas. <br>
<br>
I changed most places according to your
suggestions (for the rest few, I <br>
did <br>
not change because it may slightly change
the meaning of the definition). <br>
Also, I went through the file to change some
similar common errors that <br>
occurred. <br>
<br>
Several issues: <br>
<br>
(1) parallel gc should be lower case or
upper case? <br>
As I know and, perhaps, someone told me, the
term "parallel gc" sometimes <br>
refers to a more generic terminology rather
than ParallelGC or <br>
ParallelOldGC, <br>
if it doesn't imply these two. Basically,
any GC algorithms that can be <br>
parallelized could fall into this category.
Right now, we probably refer <br>
"parallel gc" to ParallelGC or
ParallelOldGC. But, we reserve the right to
<br>
change it to something else. That's why
"fuzziness" exists here. I guess <br>
the <br>
same way to deal with "concurrent gc". So, I
didn't make these words upper <br>
cased. <br>
<br>
(2) Identification: <br>
The general rule is to follow the example
(product()'s identification). The <br>
whole file checked and space adjusted
accordingly. <br>
<br>
(3) Line breaks <br>
A space should be provided at the line
breaks if multiple lines is needed. <br>
Some lines that are slightly longer than
normal length is kept untouched as <br>
long as they read well. <br>
<br>
*(4) Why does the VM option hashCode have
lower case initial? <br>
The direct answer is "I don't know". (But it
is understandable that <br>
hashCode <br>
is "legacy" such that it has it own style)
Anyone knows the answer? <br>
</blockquote>
<br>
I don't know for sure either but might be
because of the Java code <br>
<br>
public int*hashCode*() <br>
<br>
<br>
Jon <br>
<br>
<blockquote type="cite"> <br>
Thanks. <br>
Tao <br>
<br>
On 3/21/13 3:09 PM, Tao Mao wrote: <br>
<blockquote type="cite">8010506 Typos and
errors in gc-related flags in globals.hpp
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-8010506">https://jbs.oracle.com/bugs/browse/JDK-8010506</a>
<br>
<br>
webrev: <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/8010506/webrev.00/">http://cr.openjdk.java.net/~tamao/8010506/webrev.00/</a>
<br>
<br>
Changeset: <br>
NewRatio should defined as <br>
product(uintx, NewRatio, <br>
2,
\ <br>
"Ratio of old/new generation
sizes") \ <br>
instead of <br>
product(uintx, NewRatio, <br>
2,
\ <br>
"Ratio of new/old generation
sizes") \ <br>
<br>
PLUS other typos found by Jesper. <br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
</body>
</html>