RFR (7xS): 8177707: Specialize G1RemSet::refine_card for concurrent/during safepoint refinement

[Thomas Schatzl]

Hi Sangheon,

  thanks for your review.

On Thu, 2017-05-04 at 15:09 -0700, sangheon wrote:
> Review for 8177707.
> 
> On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
[...]
> > 
> > 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.
> Looks good, just minor comments.
> 
> /src/share/vm/gc/g1/g1RemSet.hpp
> 152   // Refine the card corresponding to "card_ptr". Returns if the 
> given card contains Second sentence seems incomplete. Returns "true"
> if the given card contains?

Fixed.

> 
>   154   bool refine_card_during_gc(jbyte* card_ptr,
> Do we really need to rename refine_card()? Couldn't be refine_card()
> and refine_card_concurrently()?

I prefer to have the verbose names.

> --------------------------------
> src/share/vm/gc/g1/g1RemSet.cpp
>   549 void G1RemSet::refine_card_concurrently(jbyte* card_ptr,
>   550                            uint worker_i) {
> line 550 needs alignment update.
> 

Fixed.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.1/ (full)

Thanks,
  Thomas