RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

[Kim Barrett]

> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>   I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
> 
> […]
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
> 
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.

------------------------------------------------------------------------------ 
src/share/vm/gc/g1/g1RemSet.cpp
 588   // While we are processing RSet buffers during the collection, we
...
 598   assert(!r->in_collection_set(), "Concurrent refinement should not encounter cards in the collection set.");

The comment starting at line 588 seems like it needs updating to
account for line 598 being changed from an "if" to an "assert".

However, I'm not sure this change change to an assert is correct,
because of the possibility of stale cards.

------------------------------------------------------------------------------ 
src/share/vm/gc/g1/g1RemSet.cpp 
 636   HeapWord* scan_limit = r->top();

I think the commentary about this initialization is important and
ought not be deleted.

Similarly for the use of scan_top() in the gc_active case.

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp 
 688 bool G1RemSet::refine_card_during_gc(jbyte* card_ptr,

I sure hope a lot of the near duplication with the concurrent version
is going to go away in one of the later changes in this stack.

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1RemSet.cpp 
 777   _conc_refine_cards++;

Since this is not in "concurrent" refinement, is it still correct to
include it in this count?  Maybe the variable name is not right?  If
so, I'd be okay with a variable rename in a later change if that makes
management of this stack of changes easier.

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1OopClosures.inline.hpp
 114 inline static void check_obj_during_refinement(T* p, oop const obj) {

Just "inline" is sufficient, no "static" needed.

------------------------------------------------------------------------------