RFR: 8142402: G1 should not redirty cards in free regions

[Thomas Schatzl]

Hi,

On Thu, 2015-11-26 at 15:24 +0100, Erik Helin wrote:
> On 2015-11-25, Erik Helin wrote:
> > On 2015-11-11, Thomas Schatzl wrote:
> > > Hi,
> > > 
> > > On Tue, 2015-11-10 at 16:36 +0100, Erik Helin wrote:
> > > > And StefanK spotted that we no longer need to use
> > > > G1CollectedHeap::heap() in G1RedirtyLoggedCardsTasks since we store a
> > > > pointer to G1CollectedHeap in the field _g1h. Please see new webrevs:
> > > > 
> > > > - incremental
> > > > http://cr.openjdk.java.net/~ehelin/8142402/webrev.01-02/
> > > > - full:
> > > > http://cr.openjdk.java.net/~ehelin/8142402/webrev.02/
> > > > 
> > > > Thanks,
> > > > Erik
> > 
> > Thomas, Stefan,
> > 
> > I discovered (after some more testing) that the condition for when to
> > dirty cards is slightly wrong in this patch.
> > 
> > Since free_collection_set is run *after* redirty_logged_cards
> > (which is being called from post_evacuate_collection_set), the regions
> > will not be free when they are being processed by redirty_logged_cards.
> > Therefore, we should only dirty cards if the region won't be freed by
> > free_collection_set. A region will be freed if it is in the
> > collection set and has not had an evacuation failure.
> > 
> > Please see updated webrevs:
> > - inc: http://cr.openjdk.java.net/~ehelin/8142402/webrev.02-03/
> > - full: http://cr.openjdk.java.net/~ehelin/8142402/webrev.03/
> 
> I discussed this patch with Thomas and Mikael offline and we decided to
> add some more documentation as to why this is needed. I have introduced
> two methods and also added some comments which hopefully make things
> clearer.
> 
> Please see new webrevs:
> - inc: http://cr.openjdk.java.net/~ehelin/8142402/webrev.03-04/
> - full: http://cr.openjdk.java.net/~ehelin/8142402/webrev.04/
> 

  looks good.

Thomas