RFR (S): 8067339: PLAB reallocation might result in failure to allocate object in that recently allocated PLAB
bengt.rutisson at oracle.com
Mon Aug 17 09:10:24 UTC 2015
"It would be nice if the logic could be in two places instead of one. "
I claim 'vacation brain' since I just got back from vacation. I of
course meant that the logic should be in one place. :)
On 2015-08-17 10:50, Bengt Rutisson wrote:
> Hi Thomas,
> On 2015-08-14 10:29, Thomas Schatzl wrote:
>> Hi all,
>> can I have reviews for the following small fix: if G1 reallocates a
>> PLAB for an object of size X, it allocates a new PLAB of size Y.
>> PLABs are required to contain enough space for the object plus some
>> alignment reserve; so if X >= Y - alignment-reserve, G1 failed to
>> allocate this object it just allocated the PLAB for into that PLAB,
>> resulting in an evacuation failure although there is actually enough
>> As far as I know this has been a rare occurrence until now because
>> current PLAB sizing tended to maximize PLAB sizes, and it is rare to
>> have such large objects, but with future changes that actually try to
>> size the PLABs so that the expected waste is kept, I noticed a few cases
>> of that issue occurring. So this needs to be fixed.
>> The fix is to consider the required alignment reserve in the PLAB
>> allocation request.
>> This is a day one G1 bug.
>> JPRT, vm.gc testlist, lots of testing.
> Interesting bug. Nice find!
> I agree that you fix solves the problem. I'm just wondering if we
> could make the dependency on the alignment reserve between
> G1PLABAllocator and PLAB more explicit. With your fix,
> G1PLABAllocator::allocate_direct_or_new_plab() has to know exactly
> what PLAB::set_buf() does to fix the sizes correctly. It would be nice
> if the logic could be in two places instead of one.
> Not sure exactly how to do that. One way would be to move the
> alignment reserve fixup up to
> G1PLABAllocator::allocate_direct_or_new_plab(). It could add the
> alignment reserve to the size it passes to par_allocate_during_gc().
> Then PLAB::set_buf() could just assert that the size if the PLAB is at
> least the size of the alignment reserve. Potentially this would look
> nicer if PLAB::set_buf() and PLAB::set_word_size() were combined into
> one method that also takes the alignment reserve.
> I realize that this would affect CMS also, unless the new changes are
> contained in G1PLAB. But looking at
> ParScanThreadState::alloc_in_to_space_slow(), I think that CMS may
> have the same issue, right? So, maybe it is good to fix it for both
More information about the hotspot-gc-dev