RFR: 8077836: Make sure G1ParGCAllocBuffer are marked as retired

[Kim Barrett]

On Apr 20, 2015, at 11:07 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
> 
> Thanks for reviewing Thomas,
> 
> On 2015-04-20 17:00, Thomas Schatzl wrote:
>> Hi Stefan,
>> 
>> On Wed, 2015-04-15 at 15:08 +0200, Stefan Johansson wrote:
>>> Hi,
>>> 
>>> Please review this fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8077836
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8077836/hotspot.00/
>>> 
>>> Summary:
>>> At the end of a G1 gc a call to G1ParGCAllocator::retire_alloc_buffers
>>> is made, in this code we should make sure all alloc buffers are retired.
>>> This is currently handle by calling flush_and_retire_stats for each
>>> active buffer. Because this method is only implemented for
>>> ParGCAllocBuffer and not for G1ParGCAllocBuffer we currently miss
>>> updating the _retired field in G1ParGCAllocBuffer.
>>> 
>>> The _retired field is checked in a guarantee in the G1ParGCAllocBuffer
>>> destructor, but this destructor is not usually called because we are
>>> missing a virtual destructor for G1ParGCAllocator.
>>> 
>>> This patch adds this destructor and implements
>>> G1ParGCAllocBuffer::flush_and_retire_stats by calling the version in
>>> ParGCAllocBuffer and setting the _retired field to true.
>>> 
>>> Testing:
>>> Manual verification that G1 always fails with the new destructor and
>>> that the fix avoids this.
>> Looks good. One thing that might be interesting to add for consistency
>> is to add the early-out like in retire() if the PLAB has already been
>> retired.
>> It does not seem to hurt to leave it out, but for consistency it might
>> be desirable.
> I thought of this too, but left it out because the change would then change the behavior if someone called flush_and_retire_stats twice and I wanted to avoid that in this change.
>> Also the change needs to be rebased to the current repository.
> Here's a rebased change:
> http://cr.openjdk.java.net/~sjohanss/8077836/hotspot.01/

Can you hold off on this for https://bugs.openjdk.java.net/browse/JDK-8078193 and the corrected push?