<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">I'm trying to answer these questions.
Please see inline. Thanks.<br>
Tao<br>
<br>
On 2/8/13 8:17 AM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">
<br>
Hi Tao,
<br>
<br>
I think the change looks good and makes sense.
<br>
<br>
But I have a hard time estimating the implications of this change.
<br>
<br>
It seems like MaxGCMinorPauseMillis is only used by CMS and
ParallelScavenge. </blockquote>
You are right. Before the fix, four occurrences below. (The one in
collectorPolicy.cpp is suspected to be wrong)<br>
<br>
MaxGCMinorPauseMillis is introduced in different kinds of classes
for CMS and PS: ConcurrentMarkSweep<b>Policy</b> and ParalleScavenge<b>Heap</b>.<br>
<br>
$ grep -nr "MaxGCMinorPauseMillis"
src/src/share/vm/gc_implementation/concurrentMarkSweep/cmsCollectorPolicy.cpp:87:
double max_gc_minor_pause_sec = ((double)
MaxGCMinorPauseMillis)/1000.0;<br>
src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp:156:
double max_gc_minor_pause_sec = ((double)
MaxGCMinorPauseMillis)/1000.0;<br>
src/share/vm/memory/collectorPolicy.cpp:170: const double
max_gc_minor_pause_sec = ((double) MaxGCMinorPauseMillis)/1000.0;<br>
src/share/vm/runtime/globals.hpp:2043: product(uintx,
MaxGCMinorPauseMillis, max_uintx, \<br>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">For
both of those the default for MaxGCMinorPauseMillis is the same as
MaxGCPauseMillis so it should be fine.
<br>
</blockquote>
How did you infer that the default value for MaxGCMinorPauseMillis
is same as MaxGCPauseMillis? I didn't see such an indication in the
code.<br>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">
<br>
But what happens if a customer is running with
-XX:MaxGCMinorPauseMillis=1000 but not setting MaxGCPauseMillis on
the command line? What are the implications of your change for
such runs? I don't think it is very common. Just thinking.
<br>
</blockquote>
For ParallelScavenge, customers should distinguish the two flags.<br>
For CMS, I wonder whether we have fully implemented adaptive sizing
policy. Jon, can you comment on this?<br>
In either case, customers should be able to tell apart.<br>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">
<br>
Also, a more common case is probably to run with only
-XX:MaxGCPauseMillis set on the command line. How will the
adaptive sizing behave differently compared to before?
<br>
</blockquote>
The fix only changed the value in the routine
GenCollectorPolicy::initialize_size_policy, which is believed to be
called within<br>
the routine of GenCollectedHeap::post_initialize(). I'm not sure
whether this routine can change sizing behaviors somewhere.<br>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">
<br>
Looking closer at the code it looks like CMS and ParallelScavenge
are actually using CMSAdaptiveSizePolicy and PSAdaptiveSizePolicy
respectively. Both of these already pass the correct value to the
constructor of the super class AdaptiveSizePolicy. So, this bug is
currently benign. The code that you are changing is actually not
being used. Is this a correct conclusion?
<br>
</blockquote>
See above. Whether the code I'm changing is being used depends upon
whether GenCollectedHeap::post_initialize() is called anywhere. But
I can't determine that right now.<br>
<br>
Other than that, the fix should correct the bug while preserving
everything else.<br>
<blockquote cite="mid:51152519.9030109@oracle.com" type="cite">
<br>
Thanks,
<br>
Bengt
<br>
<br>
On 2/8/13 3:42 AM, Tao Mao wrote:
<br>
<blockquote type="cite">8007764: Wrong initialized value of
max_gc_pause_sec for an instance of class AdaptiveSizePolicy
<br>
<a class="moz-txt-link-freetext" href="https://jbs.oracle.com/bugs/browse/JDK-8007764">https://jbs.oracle.com/bugs/browse/JDK-8007764</a>
<br>
<br>
webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tamao/8007764/webrev.00/">http://cr.openjdk.java.net/~tamao/8007764/webrev.00/</a>
<br>
<br>
changeset:
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>