RFR: 7169803: Usage of pretenured value is not correct

[Jon Masamitsu]

On 06/11/2015 12:47 AM, Tao Mao wrote:
> Hi Jon,
>
> 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?

Tao,

Yes, you're right about the definitions.  I think then the question is
what do you want to passed to

avg_promoted()->sample()

I would say that you want to pass promoted as calculated by

489       size_t promoted = old_gen->used_in_bytes() - old_gen_used_before;

I think the bug is that _avg_pretenured->padded_average() should not be
passed to sample() in the original code

1307 avg_promoted()->sample(promoted + _avg_pretenured->padded_average());

Since _avg_pretenured() should have been calculated using bytes but was 
using
words, the bug was had less of an affect.  Also the amount pretenured 
was likely
small in the majority of cases.  And the end affect would be that the number
used for promoted would be an over-estimate and just make the ergonomics 
more
conservative.

David,

If you could provide me with jprt binaries for before and after your fix,
I'll do some experiments to see if there are any change in the number
of GC (young and full) to see if we're changing the ergonomics.  It
could be that the bug is a "lucky" mistake that makes things better.
Unlikely but I'd like to look.

Thanks.

Jon


>
> Thanks.
> Tao Mao
>
>
>
> On Wed, Jun 10, 2015 at 6:48 PM, Jon Masamitsu 
> <jon.masamitsu at oracle.com <mailto:jon.masamitsu at oracle.com>> wrote:
>
>
>
>     On 6/10/2015 5:51 PM, Tao Mao wrote:
>>     I think David's right. Promoted objects and pretenured objects
>>     are different guys. This would resolve the second issue in the
>>     ticket JDK-7169803.
>
>     Tao,
>
>     How would you define each?
>
>     Jon
>
>
>>
>>     Thanks.
>>     Tao Mao
>>
>>     On Wed, Jun 10, 2015 at 5:08 PM, Jon Masamitsu
>>     <jon.masamitsu at oracle.com <mailto:jon.masamitsu at oracle.com>> wrote:
>>
>>
>>
>>         On 6/10/2015 1:30 AM, David Lindholm wrote:
>>
>>             Jon,
>>
>>             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.
>>
>>
>>         Aren't "promoted" and
>>         "total_pretenured_since_last_promotion()" approximately the same
>>         thing?  In share/vm/gc/parallel/psScavenge.cpp
>>
>>         489       size_t promoted = old_gen->used_in_bytes() -
>>         old_gen_used_before;
>>         490  size_policy->update_averages(_survivor_overflow,
>>         survived, promoted);
>>
>>         so "promoted" is the change in the old gen between the before
>>         and after the
>>         young gen collection.
>>
>>         "total_pretenured_size_last_promotion()" is
>>
>>          258   void tenured_allocation(size_t size) {
>>          259     _avg_pretenured->sample(size);
>>          260  add_pretenured_since_last_promotion(size)
>>
>>
>>         which seems to me to be calculating the same thing (sum of
>>         allocations into the old gen).
>>
>>         Not so?
>>
>>         Jon
>>
>>
>>
>>
>>             Thanks,
>>             David
>>
>>             On 2015-06-09 21:37, Jon Masamitsu wrote:
>>
>>                 David,
>>
>>                 http://cr.openjdk.java.net/~david/JDK-7169803/webrev.00/src/share/vm/gc/parallel/psAdaptiveSizePolicy.cpp.frames.html
>>                 <http://cr.openjdk.java.net/%7Edavid/JDK-7169803/webrev.00/src/share/vm/gc/parallel/psAdaptiveSizePolicy.cpp.frames.html>
>>
>>
>>                 1308  avg_promoted()->sample(promoted +
>>                 total_pretenured_since_last_promotion());
>>
>>
>>                 Is including both "promoted" and
>>                 "total_pretenured_since_last_promotion()"
>>                 double counting?
>>
>>                 Jon
>>
>>                 On 06/09/2015 02:06 AM, David Lindholm wrote:
>>
>>                     Hi,
>>
>>                     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.
>>
>>                     Webrev:
>>                     http://cr.openjdk.java.net/~david/JDK-7169803/webrev.00/
>>                     <http://cr.openjdk.java.net/%7Edavid/JDK-7169803/webrev.00/>
>>                     Bug: https://bugs.openjdk.java.net/browse/JDK-7169803
>>
>>
>>                     Testing: Passed JPRT
>>
>>
>>                     Thanks,
>>                     David
>>
>>
>>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150611/1d587bd2/attachment.htm>