RFR: 8144505: Change G1ParCopyHelper to inherit OopClosure

[Mikael Gerdin]

Hi Stefan,

On 2015-12-02 16:55, Stefan Johansson wrote:
> Hi,
>
> Please review this change for:
> https://bugs.openjdk.java.net/browse/JDK-8144505
>
> Webrev:
> http://cr.openjdk.java.net/~sjohanss/8138888/8144505/hotspot.00/

   88 // Add back base class for metadata
   89 class G1ParCopyHelper : public OopClosure {
   90 protected:
   91   uint _worker_id;              // Cache value from par_scan_state.
   92   Klass* _scanned_klass;
   93   G1CollectedHeap* _g1;
   94   ConcurrentMark* _cm;
   95   G1ParScanThreadState* _par_scan_state;
   96

That's a weird ordering of the fields, why not just move the heap an par 
scan state pointers to before the other fields, just as if they were 
still in the parent class?

Otherwise it looks good.
/Mikael

>
> 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:
> G1ParCopyClosure is only used as a normal OopClosure. It inherits from
> G1ParCopyHelper which in turn inherits G1ParClosureSuper.
> G1ParClosureSuper overrides apply_to_weak_ref_discovered_field() to
> return true but since G1ParCopyClosure is never used with oop_iterate()
> it doesn't need to inherit this functionality. G1ParCopyHelper can
> instead be changed to inherit directly from OopClosure, and this way we
> have one closure less depending on apply_to_weak_ref_discovered_field().
>
> Testing:
> * JPRT
> * RBT
>
> Thanks,
> Stefan
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8138888