<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Tao,<br>
<br>
Thanks for the review!<br>
<br>
<br>
/David<br>
<br>
<div class="moz-cite-prefix">On 2015-06-15 22:37, Tao Mao wrote:<br>
</div>
<blockquote
cite="mid:CANrGW1w6OVmKXV6bd2hJBXzCge4TFdN=+VoqVJHc1SWGHoxf_w@mail.gmail.com"
type="cite">
<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 moz-do-not-send="true"
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 moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Edavid/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
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 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
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>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>