RFR(M): 7009266: G1: assert(obj->is_oop_or_null(true )) failed: Error

[Tom Rodriguez]

Looks good.

tom

On Apr 4, 2011, at 4:43 PM, John Cuthbertson wrote:

> Hi Everyone,
> 
> A new webrev for this fix can be found at: http://cr.openjdk.java.net/~johnc/7009266/webrev.5/
> 
> The changes in this revision include:
> 
> * Revised barrier set calls in JNI_GetObjectField and Unsafe_getObject so that the value that is returned is the value that gets recorded in an SATB buffer. The code in the previous revision re-read the value of the field potentially causing the value the is logged to be different from that returned.
> 
> * Added the static checks, suggested by Tom, in library_call.cpp.
> 
> * Removed the complicated control flow from the C1 LIR implementation of Unsafe.getObject. Instead the checks have been moved into a code stub while some static checks have been added.
> 
> * Re-enabled reference discovery (and therefore reference processing) during concurrent marking as a result of pushing the changes for 7020042 to hs21.
> 
> Thanks,
> 
> JohnC
> 
> On 03/28/11 10:35, John Cuthbertson wrote:
>> Hi Everyone,
>> 
>> A new webrev with changes based upon comments from Tom can be found at: http://cr.openjdk.java.net/~johnc/7009266/webrev.4/.
>> 
>> The latest changes include inserting a suitably guarded barrier call in case the referent field of a Reference object is being read/fetched using JNI, reflection, or Unsafe.
>> 
>> Thanks,
>> 
>> JohnC
>> 
>> On 3/11/2011 5:34 PM, John Cuthbertson wrote:
>>> Hi Everyone,
>>> 
>>> I'm looking for a few of volunteers to review the changes that fix this assertion failure. The latest changes can be found at: http://cr.openjdk.java.net/~johnc/7009266/webrev.3/ and include changes based upon earlier internal reviews. The earlier changes are also on cr.openjdk.java.net for reference.
>>> 
>>> Background:
>>> The G1 garbage collector includes a concurrent marking algorithm that makes use of snapshot-at-the-beginning or SATB. With this algorithm the GC will mark all objects that are reachable at the start of marking; objects that are allocated since the start of marking are implicitly considered live. In order to populate the "snapshot" of the object graph that existed at the start of marking, G1 employs a write barrier. When an object is stored into another object's field the write-barrier records the previous value of that field as it was part of the "snapshot" and concurrent marking will trace the sub-graph that is reachable from this previous value.
>>> 
>>> Unfortunately, in the presence of Reference objects, SATB might not be sufficient to mark a referent object as live. Consider that, at the start of marking, we have a weakly reachable object i.e. an object where the only pointer to that object. If the referent is obtained from the Reference object and stored to another object's field (making the referent now strongly reachable and hence live) the G1 write barrier will record the field's previous value but not the value of the referent.
>>> 
>>> If the referent object is strongly reachable from some other object that will be traced by concurrent marking, _or_ there is a subsequent assignment to the field where we have written the referent (in which case we record the previous value - the referent - in an SATB buffer) then the referent will be marked live. Otherwise the referent will not be marked.
>>> 
>>> That is the issue that was causing the failure in this CR. There was a Logger object that was only reachable through a WeakReference at the start of concurrent marking. During marking the Logger object is obtained from the WeakReference and stored into a field of a live object. The G1 write barrier recorded the previous value in the field (as it is part of the snapshot at the start of marking). Since there was no other assignment to the live object's field and there was no other strong reference to the Logger object, the Logger object was not marked. At the end of concurrent marking the Logger object was considered dead and the link between the WeakReference and the Logger was severed by clearing the referent field during reference processing.
>>> 
>>> To solve this (entirely in Hotspot and causing a performance overhead for G1 only) it was decided that the best approach was to intrinsify the Reference.get() method in the JIT compilers and add new interpreter entry points so that the value in the referent field will be recorded in an SATB buffer by the G1 pre-barrier code.
>>> 
>>> The changes for Zero and the C++ interpreters are place holder routines but should be straight forward to implement.
>>> 
>>> None of the individual changes is large - they are just well distributed around the JVM. :)
>>> 
>>> Testing: white box test; eyeballing the generated compiled and interpreter code; the failing Kitchensink big-app on x86 (32/64 bit), sparc (32/64 bit), Xint, Xcomp (client and server), with and without G1; the GC test suite with and without G1; and jprt.
>>> 
>>> Thanks and regards,
>>> 
>>> JohnC
>> 
>