RFR (XXS): 8005857: assert in GC_locker from PSOldGen::expand with -XX:+PrintGCDetails and Verbose

[John Cuthbertson]

Hi Thomas,

The change looks god to me.

JohnC

On 3/29/2013 1:51 AM, Thomas Schatzl wrote:
> Hi all,
>
>    please review the following change for 8005857:
>
> In this issue, code checks whether the gc locker is active during heap
> expansion to print a message that instead of gc the heap has been expanded.
>
> The code used the method GCLocker::is_active() to get the gc locker
> state, but is_active() asserts when the VM is currently not at a
> safepoint and no GC is actually needed.
> This assert does not hold in all cases while expanding the heap. E.g. heap
> expansion may be done outside of a safepoint when satisfying an allocation
> request for a large object. Heap expansion may occur also when no GC is
> pending in the gc locker.
>
> Imho the previous use of is_active() was wrong not only because of the assert.
> The message text notifies of the avoidance of a gc. However, the condition did
> not check whether the gc locker indicated the need for a gc, i.e. did not
> check the GCLocker::needs_gc() predicate.
>
> This change uses GCLocker::is_active_and_needs_gc() instead of
> GCLocker::is_active(). Which does not check whether the vm is at a safepoint,
> and also checks whether a gc is actually pending.
>
> Jon M. suggested this fix.
>
> JIRA:
> https://jbs.oracle.com/bugs/browse/JDK-8005857
>
> bugs.sun:
> http://bugs.sun.com/view_bug.do?bug_id=8005857
>
> Testing:
> jprt, kitchensink, runthese jck tests (which contains the test case
> where this bug has been reported)
>
> Thanks,
> Thomas
>
>
>
>