<div dir="ltr"><div>The code seems to only consider survisor_size and promoted_size and ignore pretenured size for most part, except here</div><div><br></div><div>avg_promoted()->sample(promoted + _avg_pretenured->padded_average());<br></div><div><br></div><div>The naming of avg_promoted() is probably not correct either if we want to make it consistent with what it takes in.</div><div><br></div><div>To take all involved sizes into considerations, we need to fix all over the places when we make decisions based on the sizes.</div><div><br></div><div><br></div><div><br></div><div>Jon, you could be right. The chances are GCSizePolicy is just doing all right and pretenured size won't matter too much whether it's in the calculation or not. That needs to be instrumented, of course.</div><div><br></div><div>Thanks.</div><div>Tao</div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 11, 2015 at 10:22 AM, Jon Masamitsu <span dir="ltr"><<a href="mailto:jon.masamitsu@oracle.com" target="_blank">jon.masamitsu@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class="">
<br>
<br>
<div>On 06/11/2015 12:47 AM, Tao Mao wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Jon,
<div><br>
</div>
<div>The definition is: "Promoted" is objects that are promoted
(or tenured) during yGC while "PRE-tenured" is objects that
are allocated directly in old gen. I believe this is so,
right?</div>
</div>
</blockquote>
<br></span>
Tao,<br>
<br>
Yes, you're right about the definitions. I think then the question
is<br>
what do you want to passed to<br>
<br>
avg_promoted()->sample()<br>
<br>
I would say that you want to pass promoted as calculated by<span class=""><br>
<br>
489Â Â Â Â size_t promoted = old_gen->used_in_bytes() -
old_gen_used_before;<br>
<br></span>
I think the bug is that <span>_avg_pretenured->padded_average()</span>
should not be<br>
passed to sample() in the original code<br>
<br>
<span>1307 avg_promoted()->sample(promoted +
_avg_pretenured->padded_average());</span><br>
<br>
Since _avg_pretenured() should have been calculated using bytes but
was using<br>
words, the bug was had less of an affect. Also the amount
pretenured was likely<br>
small in the majority of cases. And the end affect would be that
the number<br>
used for promoted would be an over-estimate and just make the
ergonomics more<br>
conservative. <br>
<br>
David,<br>
<br>
If you could provide me with jprt binaries for before and after your
fix,<br>
I'll do some experiments to see if there are any change in the
number<br>
of GC (young and full) to see if we're changing the ergonomics. It<br>
could be that the bug is a "lucky" mistake that makes things better.<br>
Unlikely but I'd like to look.<br>
<br>
Thanks.<div><div class="h5"><br>
<br>
Jon<br>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thanks.</div>
<div>Tao Mao</div>
<div><br>
</div>
<div><br>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Jun 10, 2015 at 6:48 PM, Jon
Masamitsu <span dir="ltr"><<a href="mailto:jon.masamitsu@oracle.com" target="_blank">jon.masamitsu@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span> <br>
<br>
<div>On 6/10/2015 5:51 PM, Tao Mao wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">I think David's right. Promoted objects
and pretenured objects are different guys. This
would resolve the second issue in the ticket
JDK-7169803.</div>
</blockquote>
<br>
</span> Tao,<br>
<br>
How would you define each?<span><font color="#888888"><br>
<br>
Jon</font></span>
<div>
<div><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thanks.</div>
<div>Tao Mao</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Jun 10, 2015 at
5:08 PM, Jon Masamitsu <span dir="ltr"><<a href="mailto:jon.masamitsu@oracle.com" target="_blank">jon.masamitsu@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
<br>
On 6/10/2015 1:30 AM, David Lindholm wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Jon,<br>
<br>
Thanks for taking a look at this. No, I
don't think this will lead to double
counting. This calculation is already
there, it is just buggy. Before this patch
the code added the total amount of
promoted memory this collection with the
average size of the pretenured objects,
and used this as a sample. Now the code
adds the total amount of promoted memory
with the total size of the pretenured
objects since last collection, and uses
this as a sample instead.<br>
</blockquote>
<br>
</span> Aren't "promoted" and
"total_pretenured_since_last_promotion()"
approximately the same<br>
thing? In share/vm/gc/parallel/psScavenge.cpp<br>
<br>
489Â Â Â Â size_t promoted =
old_gen->used_in_bytes() -
old_gen_used_before;<br>
490Â Â Â
 size_policy->update_averages(_survivor_overflow,
survived, promoted);<br>
<br>
so "promoted" is the change in the old gen
between the before and after the<br>
young gen collection.<br>
<br>
"total_pretenured_size_last_promotion()" is<br>
<br>
 258  void tenured_allocation(size_t size) {<br>
 259   _avg_pretenured->sample(size);<br>
 260 Â
 add_pretenured_since_last_promotion(size)<br>
<br>
<br>
which seems to me to be calculating the same
thing (sum of allocations into the old gen).<br>
<br>
Not so?<span><font color="#888888"><br>
<br>
Jon</font></span>
<div>
<div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br>
<br>
Thanks,<br>
David<br>
<br>
On 2015-06-09 21:37, Jon Masamitsu
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> David,<br>
<br>
<a href="http://cr.openjdk.java.net/%7Edavid/JDK-7169803/webrev.00/src/share/vm/gc/parallel/psAdaptiveSizePolicy.cpp.frames.html" target="_blank">http://cr.openjdk.java.net/~david/JDK-7169803/webrev.00/src/share/vm/gc/parallel/psAdaptiveSizePolicy.cpp.frames.html</a>
<br>
<br>
1308Â
 avg_promoted()->sample(promoted +
total_pretenured_since_last_promotion());<br>
<br>
<br>
Is including both "promoted" and
"total_pretenured_since_last_promotion()"<br>
double counting?<br>
<br>
Jon<br>
<br>
On 06/09/2015 02:06 AM, David Lindholm
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Hi,<br>
<br>
Please review this patch that
corrects the usage of the pretenured
value. There were 2 issues: words
and bytes were mixed up and the
addition was done with the wrong
value. See bug for details.<br>
<br>
Webrev: <a href="http://cr.openjdk.java.net/%7Edavid/JDK-7169803/webrev.00/" target="_blank">http://cr.openjdk.java.net/~david/JDK-7169803/webrev.00/</a><br>
Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-7169803" target="_blank">https://bugs.openjdk.java.net/browse/JDK-7169803</a><br>
<br>
<br>
Testing: Passed JPRT<br>
<br>
<br>
Thanks,<br>
David<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>