RFR: 8144052: mark_card_deferred does not need to check g1_young_gen

[Mikael Gerdin]

Hi,

On 2015-11-25 17:56, Erik Helin wrote:
> On 2015-11-25, Thomas Schatzl wrote:
>> Hi Erik,
>
> Hi Thomas,
>
> thanks for reviewing!
>
>> On Wed, 2015-11-25 at 16:24 +0100, Erik Helin wrote:
>>> Hi all,
>>>
>>> this patch removes an unneeded if statement from
>>> G1SATBCardTableModRefBS::mark_card_deferred. mark_card_deferred is only
>>> called from two places:
>>> - UpdateRSetDeferred::do_oop_nv
>>> - G1ParScanThreadState::update_rs
>>>
>>> In the case of UpdateRSetDeferred::do_oop_nv, the check is always false
>>> because we clear the card table in
>>> cleanup_after_oops_into_collection_set_do prior to calling
>>> remove_self_forwarding_pointers.
>>>
>>> In the case of G1ParScanThreadState::update_rs, then we can update the
>>> check whether to mark a card or not to check !from->is_young instead of
>>> !from->is_survivor.
>>>
>>> Therefore, the if statement:
>>> if (val == g1_young_gen) {
>>>         return false;
>>> }
>>> can be removed from G1SATBCardTableModRefBS::mark_card_deferred.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8144052
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~ehelin/8144052/webrev.00/
>>>
>>> Testing:
>>> - GC test suite with full verification
>>
>>   looks good, except that I would like to ask you to add an assertion
>> with some good message to G1SATBCardTableModRefBS::mark_card_deferred
>> that checks that we are not calling it on a young gen region.
>
> Unfortunately we can't add such an assert because UpdateRSetDeferred
> will call mark_card_deferred for cards corresponding to young regions
> because we might have gotten an evacuation failure in a young region.
>
> So, UpdateRSetDeferred will call mark_card_deferred for Old and Young
> regions (never Survivor). G1ParScanThreadState will call
> mark_card_deferred for Old regions only (never Young nor Survivor).

Sounds reasonable, Reviewed.
/Mikael

>
> Thanks,
> Erik
>
>> Thanks,
>>    Thomas
>>