RFR: 8144908: Remove apply_to_weak_ref_discovered_field override for UpdateRSOopClosure

[Stefan Johansson]

On 2015-12-08 22:16, Kim Barrett wrote:
> On Dec 8, 2015, at 3:34 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>> Stefan,
>>
>> At line 41 in "share/vm/oops/instanceRefKlass.inline.hpp"
>>
>>     38  void InstanceRefKlass::oop_oop_iterate_ref_processing_specialized(oop obj, OopClosureType* closure,
>> Contains& contains) {
>>     39    T* disc_addr = (T*)java_lang_ref_Reference::discovered_addr(obj);
>>     40    if (closure->apply_to_weak_ref_discovered_field()) {
>>     41      Devirtualizer<nv>::do_oop(closure, disc_addr);
>>     42    }
>>
>> you're saying that the "next" field of "obj" is
>> non-null?  Did you try adding a guarantee to that
>> effect and run your testing (without your change,
>> of course)?
> If next is NULL then, <em>for the specific closure in question</em>, discovered must also be NULL here.  There is no point in applying closure to the NULL-valued discovered field.  There are other closures for which that relationship between next and discovered doesn’t hold, but we think it will turn out that none of them have apply_to_weak_ref_discovered_field returning true.
>
> Note that if next is non-NULL, then the closure will be applied to discovered in the normal path, and there is no need for this special case forced application (and is in fact duplicating work at present, which could be a bug on its own, except we think the special case is going away).
>
>
As Kim say, for this specific closure the assumption holds, but it's not 
true for all closures. Some time ago I did test-runs with extra debug 
code to see which closures were applied with discovered field set but 
next field NULL. The two closures that showed up was:
FilterOutOfRegionClosure
G1ParPushHeapRSClosure

They both have apply_to_weak_ref_discovered_field returning true, but to 
me it is not clear that they need it. My goal is to remove all other use 
of apply_to_weak_ref_discovered_field first and then attack those two. 
Either by somehow showing that the special case is no longer needed or 
by handling the discovered field (or actually the discovered list) some 
other way.