<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<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> 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><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> 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?
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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/8010506/webrev.04/">http://cr.openjdk.java.net/~tamao/8010506/webrev.04/</a>
<br>
<br>
CR:
<br>
<a 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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/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 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 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 class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/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>
</body>
</html>