RFR: 8183539: Remove G1RemSet::_into_cset_dirty_card_queue_set

[Thomas Schatzl]

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.

Thomas