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

[Kim Barrett]

> On Jul 6, 2018, at 9:10 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> 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

Looks good.