RFR: 7169803: Usage of pretenured value is not correct

David Lindholm david.lindholm at oracle.com
Mon Jun 15 12:07:25 UTC 2015


Tao and Jon,

I have changed the patch now so that pretenured is not part of the 
calculation at all:

http://cr.openjdk.java.net/~david/JDK-7169803/webrev.01/


Thanks,
David

On 2015-06-14 20:58, Tao Mao wrote:
> The code seems to only consider survisor_size and promoted_size and 
> ignore pretenured size for most part, except here
>
> avg_promoted()->sample(promoted + _avg_pretenured->padded_average());
>
> The naming of avg_promoted() is probably not correct either if we want 
> to make it consistent with what it takes in.
>
> To take all involved sizes into considerations, we need to fix all 
> over the places when we make decisions based on the sizes.
>
>
>
> 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.
>
> Thanks.
> Tao
>
>
>
>
>
> On Thu, Jun 11, 2015 at 10:22 AM, Jon Masamitsu 
> <jon.masamitsu at oracle.com <mailto:jon.masamitsu at oracle.com>> wrote:
>
>
>
>     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/20150615/39f1b082/attachment.htm>


More information about the hotspot-gc-dev mailing list