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

[sangheon]

On 05/05/2017 04:49 AM, Thomas Schatzl wrote:
> 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.
OK.

>
>> --------------------------------
>> 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)
webrev.1 seems good to me.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>