RFC: Patch for 8203848: Missing remembered set entry in j.l.ref.references after JDK-8203028

[Thomas Schatzl]

Hi,

On Thu, 2018-06-07 at 11:47 +0200, Thomas Schatzl wrote:

  any comments on this issue?

Thanks,
  Thomas

> Hi all,
> 
>   I would like to ask for comments on the fix for the issue handled
> in
> JDK-8203848.
> 
> In particular, the problem is that currently the "discovered" field
> of
> j.l.ref.References is managed completely opaque to the rest of the
> VM,
> which causes the described error: remembered set entries are missing
> for that reference when doing *concurrent* reference discovery.
> 
> There is no problem with liveness of objects referenced by that
> because
> a) a j.l.ref.Reference object found by reference discovery will be
> automatically kept alive and
> b) no GC in Hotspot at this time evacuates old gen objects during
> marking (and Z does not use the reference processing framework at
> all),
> so that reference in the "discovered" field will never be outdated.
> 
> However in the future, G1 might want to move objects in old gen at
> any
> time for e.g. defragmentation purposes, and I am a bit unclear about
> Shenandoah tbh :)
> 
> I see two solutions for this issue:
> - improve the access modifier so that at least the post-barrier that
> is
> responsible for adding remembered set entries is invoked on this
> field.
> E.g. in ReferenceProcessor::add_to_discovered_list_mt(), instead of
> 
> oop retest = RawAccess<>::oop_atomic_cmpxchg(next_discovered,
> discovered_addr, oop(NULL));
> 
> do a
> 
> oop retest =
> HeapAccess<AS_NO_KEEPALIVE>::oop_atomic_cmpxchg(next_discovered,
> discovered_addr, oop(NULL));
> 
> Note that I am almost confident that this only makes G1 work as far
> as
> I understand the access framework; since the previous value is NULL
> when we cmpxchg, G1 can skip the pre-barrier; maybe more is needed
> for
> Shenandoah, but I hope that Shenandoah devs can chime in here.
> 
> I tested this, and with this change the problem does not occur after
> 2000 iterations of the test.
> 
> (see the preliminary webrev at http://cr.openjdk.java.net/~tschatzl/8
> 20
> 3848/webrev/ ; only the change to referenceProcessor.cpp is relevant
> here).
> 
> - the other "solution" is to fix the remembered set verification to
> ignore this field, and try to fix this again in the future when/if G1
> evacuates old regions during marking.
> 
> Any comments?
> 
> Thanks,
>   Thomas
>