<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Looks good. Reviewed.<br>
<br>
Sorry for the late response.<br>
<br>
Jon<br>
<br>
<br>
<div class="moz-cite-prefix">On 6/15/2015 5:07 AM, David Lindholm
wrote:<br>
</div>
<blockquote cite="mid:557EBFFD.60109@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
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 moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edavid/JDK-7169803/webrev.01/">http://cr.openjdk.java.net/~david/JDK-7169803/webrev.01/</a><br>
<br>
<br>
Thanks,<br>
David<br>
<br>
<div class="moz-cite-prefix">On 2015-06-14 20:58, Tao Mao wrote:<br>
</div>
<blockquote
cite="mid:CANrGW1y17md9FOo-vDhMb1gxXBT_zeEaURVkJ21PNbwPHiH6MA@mail.gmail.com"
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 moz-do-not-send="true"
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
moz-do-not-send="true"
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
moz-do-not-send="true"
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
moz-do-not-send="true"
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
moz-do-not-send="true"
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
moz-do-not-send="true"
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>
</blockquote>
<br>
</body>
</html>