RFR (7xS): 8175554: Improve G1UpdateRSOrPushRefClosure

Thomas Schatzl thomas.schatzl at oracle.com
Tue Jun 20 08:05:47 UTC 2017


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




More information about the hotspot-gc-dev mailing list