RFR (7xS): 8175554: Improve G1UpdateRSOrPushRefClosure

[Thomas Schatzl]

Hi again,

  webrev links:
http://cr.openjdk.java.net/~tschatzl/8175554/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8175554/webrev.1/ (full)

Thomas
 
On Tue, 2017-06-20 at 10:05 +0200, Thomas Schatzl wrote:
> Hi Sangheon, others,
> 
> On Tue, 2017-05-30 at 15:15 -0700, sangheon wrote:
> > 
> > Hi Thomas,
> > 
> > On 05/05/2017 05:13 AM, Thomas Schatzl wrote:
> > > 
> > > 
> > > Hi all,
> > > 
> > >    recent reviews have made changes necessary to parts of the
> > > changeset chain.
> > > 
> > > Here is a list of links to updated webrevs. Since they have
> > > apparently not been reviewed yet, I simply overwrote the old
> > > webrevs.
> > > 
> > > JDK-8177044: Remove _scan_top from HeapRegion
> > > http://cr.openjdk.java.net/~tschatzl/8177044/webrev/
> > > 
> > > JDK-8178148: Log more detailed information about scan rs phase
> > > http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
> > > 
> > > JDK-8175554: Improve G1UpdateRSOrPushRefClosure
> > > http://cr.openjdk.java.net/~tschatzl/8175554/webrev/
> > Looks good to me.
> > I only have minor nits.
> > 
> > ------------------------------------------------------
> > src/share/vm/gc/g1/g1OopClosures.hpp
> >    78   virtual void do_oop(oop* p) { do_oop_nv(p); }
> > Misaligned with above line.
> > 
> > ------------------------------------------------------
> > src/share/vm/gc/g1/g1RemSet.hpp
> >   204                   G1UpdateOrScanRSClosure* push_heap_cl,
> > Rename to reflect new closure name?
> > 
> > ------------------------------------------------------
> > src/share/vm/gc/g1/g1RootProcessor.hpp
> > Copyright update.
> > 
> > ------------------------------------------------------
> > src/share/vm/gc/g1/g1_specialized_oop_closures.hpp
> >    45       f(G1UpdateOrScanRSClosure,_nv)         \
> > Misaligned '\'.
> > 
>   I fixed all this in addition to incorporating ErikD's comments that
> asked for factoring out two parts of the G1ParScanClosure and
> G1UpdateOrScanRSClosure that were equal now.
> 
> I did some performance testing again due to that, and also found that
> the check to filter out non-cross-region references
> in G1UpdateOrScanRSClosure::do_oop_nv() seemed faster, so I also
> reverted it to the old code.
> 
> Also in this change G1UpdateOrScanRSClosure::do_oop_nv() did not
> update
> _has_refs_into_cset as before. Fixed that as well.
> 
> Thanks,
>   Thomas
>