<div dir="ltr">The change looks good to me.<div><br></div><div>Thanks.</div><div>Tao Mao</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 15, 2015 at 5:07 AM, David Lindholm <span dir="ltr"><<a href="mailto:david.lindholm@oracle.com" target="_blank">david.lindholm@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">
Tao and Jon,<br>
<br>
I have changed the patch now so that pretenured is not part of the
calculation at all:<br>
<br>
<a href="http://cr.openjdk.java.net/~david/JDK-7169803/webrev.01/" target="_blank">http://cr.openjdk.java.net/~david/JDK-7169803/webrev.01/</a><br>
<br>
<br>
Thanks,<br>
David<div><div class="h5"><br>
<br>
<div>On 2015-06-14 20:58, Tao Mao wrote:<br>
</div>
<blockquote type="cite">
<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> <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><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><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>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>