<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Jon,<br>
<br>
Thanks for the review!<br>
<br>
<br>
/David<br>
<br>
<div class="moz-cite-prefix">On 2015-06-17 17:24, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:55819141.1000003@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
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>
</blockquote>
<br>
</body>
</html>