RFR: 8049599: MetaspaceGC::_capacity_until_GC can overflow

[Jon Masamitsu]

Erik,

I think the change is correct as you've done it but could
you consider expanding the change a bit.

The increment of the _capacity_until_GC is fine.   However,
instead of having the atomic increment try again if the
CAS fails, how about putting the retry around the call
to allocate() in expand_and_allocate().  Something like

do {
     inc_capacity_until_GC(delta_bytes)
     result = allocate(word_size)
} while result == NULL AND count <= 5

return result;

Even if the increment of _capacity_until_GC fails, the
allocate could still succeed and it is the allocate() that
is important.

I'm not sure about terminating the allocation attempt based
on a count.  The consequence of the failed allocation is
a full GC so maybe we should do something more.
Try until _capacity_until_GC hits MaxMetaspaceSize?
Maybe something like that plus a higher count.  Not sure.

Jon


On 8/20/2014 3:27 AM, Erik Helin wrote:
> Hi all,
>
> this patch fixes a problem where Metaspace::_capacityUntilGC can 
> overflow ("wrap around"). Since _capacityUntilGC is treated as a 
> size_t everywhere it used, we won't calculate with negative numbers, 
> but an eventual wrap around will still cause unnecessary GCs.
>
> The problem is solved by detecting an eventual wrap around in 
> Metaspace::incCapacityUntilGC. The overflow check means that 
> _capacityUntilGC now must be updated with a CAS. If the CAS fails more 
> than five times due to contention, no update will be done, because 
> this means that other threads must have incremented _capacityUntilGC 
> (it is decremented only during a safepoint). This also means that a 
> thread calling incCapacityUntilGC might have "its" requested memory 
> "stolen" by another thread, but incCapacityUntilGC has never given any 
> fairness guarantees.
>
> The patch also adds two functions to the WhiteBox API to be able to 
> update and read Metaspace::_capacityUntilGC from a JTREG test.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8049599
>
> Webrev:
> http://cr.openjdk.java.net/~ehelin/8049599/webrev.00/
>
> Testing:
> - JPRT
> - Aurora ad-hoc testing (on all platforms, both 32-bit and 64-bit):
>   - Kitchensink, runThese and Dacapo
>   - JTREG tests
>   - Parallel Class Loading testlist
>   - GC, runtime and compiler testlists
>   - OOM and stress testlists
> - Running newly added JTREG test
>
> Thanks,
> Erik