RFR (S): 8205426: Humongous continues remembered set does not match humongous start region one in Kitchensink

Thomas Schatzl thomas.schatzl at oracle.com
Fri Jul 6 13:10:16 UTC 2018


Hi,

On Thu, 2018-07-05 at 16:53 -0400, Kim Barrett wrote:
> > On Jul 5, 2018, at 3:16 AM, Thomas Schatzl <thomas.schatzl at oracle.c
> > om> wrote:
> > There is a new webrev at
> > 
> > http://cr.openjdk.java.net/~tschatzl/8205426/webrev.1 (full)
> > http://cr.openjdk.java.net/~tschatzl/8205426/webrev.0_to_1 (diff,
> > but
> > almost useless due to many changes)
> > 
> > That at least separates the concerns about humongous/regular region
> > a
> > bit.
> > 
> > Thanks,
> >  Thomas
> 
> I like this much better.  It eliminates the implicit logical coupling
> that the before rebuild task "knows" the liveness of the starts
> region
> is good enough, without introducing physical coupling from remset to
> concurrentmark.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.cpp
>  116   if (!r->is_old() && r->is_archive()) {
> 
> I think that should be || rather than &&.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/g1RemSetTrackingPolicy.cpp
>  111 bool G1RemSetTrackingPolicy::update_before_rebuild(HeapRegion*
> r, size_t live_bytes) {
> 
> Consider adding "assert(!r->is_humongous(), ...)".  The !r->is_old()
> will filter them out, but we shouldn't be here at all and should have
> instead called the associated update_humongous function.
> 
> -------------------------------------------------------------------
> -----------
> 

  fixed all that and Erik's suggestion.

New webrev:
http://cr.openjdk.java.net/~tschatzl/8205426/webrev.2 (full)
http://cr.openjdk.java.net/~tschatzl/8205426/webrev.1_to_2 (diff)

It passed hs-tier1-4,jdk-tier1-3

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list