RFR: 8183539: Remove G1RemSet::_into_cset_dirty_card_queue_set

[Mikael Gerdin]

Hi Erik,

On 2017-07-13 17:06, Thomas Schatzl wrote:
> Hi Erik,
> 
> On Thu, 2017-07-13 at 16:53 +0200, Erik Helin wrote:
>> On 07/04/2017 02:17 PM, Mikael Gerdin wrote:
>>>
>>> Hi Erik,
>>>
>>> Do you know if any of the tests actually would have failed if rem
>>> set
>>> reconstruction after evacuation failure didn't work properly?
>>>
>>> I'd feel safer with this change if you ran with some verification
>>> code to ensure that the into_cset queue was always useless when
>>> evac failure occurs.
>>
>> Good point, I have now run GCBasher for a very long time with:
>> -XX:+G1EvacuationFailALot -XX:G1EvacuationFailureALotCount=5
>> -XX:+VerifyBeforeGC -XX:+VerifyAfterGC
>>
>> This mean that GCBasher encounters a (forced) evacuation failure
>> every fifth GC and also runs full verification for every GC. So far
>> it has been working fine.
>>
>> I have also run all tests in the JTReg group hotspot_gc with
>> G1EvacuationFailALot set to true (in g1_globals.hpp) and
>> G1EvacuationFailureALotCount set to 5 (also in g1_globals.hpp). This
>> mean that all GC tests (including the stress tests) encountered an
>> evacuation failure every fifth GC. This also worked fine.
>>
>> I also wrote a new patch against tip (where _into_cset_dcqs is still
>> present) to do some custom verification. The contents of
>> G1RemSet::_into_cset_dcqs and G1CollectedHeap::_dirty_card_queue_set
>> should be identical after a collection. This sort-of worked :)
>>
>> The queues are *very* similar (often around 98% of the cards in
>> G1RemSet::_into_cset_dcqs are found in
>> G1CollectedHeap::_dirty_card_queue_set). The reason for the "missing
>> cards" is that cards in G1RemSet::_into_cset_dcqs comes from the
>> post-write barrier, and the post-write barrier dirties the card that
>> contains the object header (except for arrays, where it dirties the
>> field/slot). The cards in G1CollectedHeap::_dirty_card_queue_set
>> comes from G1ParScanThreadState::update_rs, and update_rs always
>> dirties the card that contains the field (*not* the header). Hence,
>> if an object crosses card boundaries, then the post-write barrier and
>> update_rs will dirty different cards. This has no impact on
>> correctness, it is like this for performance reasons (dirtying the
>> card that contains the object header leads to fewer dirty cards, but
>> we don't have quick access to the object header in update_rs).
>>
>> So, with the above, I'm fairly confident (famous last words) that
>> this patch is working :)
> 
> Thanks for this thorough investigation, sounds good.
> 
> Ship it.

+1

/Mikael
> 
> Thomas
>