RFR: 8144908: Remove apply_to_weak_ref_discovered_field override for UpdateRSOopClosure

[Kim Barrett]

On Dec 8, 2015, at 8:42 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
> 
> Hi,
> 
> Please review this change for:
> https://bugs.openjdk.java.net/browse/JDK-8144908
> 
> Webrev:
> http://cr.openjdk.java.net/~sjohanss/8138888/8144908/hotspot.00/

Looks good.

I noticed you also removed the commented out override of idempotent.  Is there a task to do anything with that?  I looked a while ago, and I think there were no classes that provided a definition other than the default one that returns false.  I think there were a couple that were commented out, like this one.  Probably there should be an RFE to either actually use it, or get rid of it.

> Background:
> When iterating objects with oop_iterate() there is a special case for InstanceRefKlass, if the closure overrides apply_to_weak_ref_discovered_field() to return true, the closure will be applied to the discovered field, even if the normal checks prevents it. This special treatment is needed in certain situations, but it is not obvious if it should be done when iterating the object. Currently only G1 closures override apply_to_weak_ref_discovered_field() and many of them seem to do it without any reason. My initial plan [1] was to remove all usage of this special casing but after discussions I realized it would be easier to see the effects if doing this one closure at a time.
> 
> Summary:
> UpdateRSOopClosure is used to re-build the remembered sets after a G1 full GC. This is done after reference discovery and enqueuing (to the pending list) has been completed, so all discovered references have their next-field set. Before the full GC is started all references discovered by the concurrent-mark reference processor is clear to allow them to be discovered during the full GC.
> 
> So at the point of applying the UpdateRSOopClosure, no references should have their discovered field set but still be active (next == null). So overriding apply_to_weak_ref_discovered_field to make sure the discovered field is handled doesn't have any effect. Either the discovered and next field are both set and thus gets handled the normal way, or the discovered field is not set and there is no reason to handle it at all.
> 
> Testing:
> * RBT
> * GC-test-suite run locally with small heap (to trigger full GC) and verification turned on.
> 
> Thanks,
> Stefan
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8138888