RFR: 8077836: Make sure G1ParGCAllocBuffer are marked as retired
Stefan Johansson
stefan.johansson at oracle.com
Mon Apr 20 15:07:31 UTC 2015
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/
Thanks,
Stefan
> Thanks,
> Thomas
>
>
More information about the hotspot-gc-dev
mailing list