RFR: 8030646: Track collection set membership in one place

[Erik Helin]

Thomas had one additional suggestion: rename register_*_with_in_cset to
register_*_with_cset. I've uploaded new patches at:

- http://cr.openjdk.java.net/~ehelin/8030646/webrev.01-02/
- http://cr.openjdk.java.net/~ehelin/8030646/webrev.02

v2:
- Rename register_*_with_in_cset (Thomas)
v1:
- Fix superfluos whitespace in assert (Thomas)
- Put back removed assert (Thomas)
- Remove the "_fast_test" suffix from register_*_with_in_cset methods
  (Thomas)

Thanks,
Erik

On 2015-03-06, Erik Helin wrote:
> On 2015-01-27, Thomas Schatzl wrote:
> > Hi,
> > 
> >   thanks for taking this over.
> 
> Thanks for reviewing and sorry for the (very) late reply. I've uploaded
> new patches at:
> 
> - http://cr.openjdk.java.net/~ehelin/8030646/webrev.00-01/
> - http://cr.openjdk.java.net/~ehelin/8030646/webrev.01
> 
> Testing:
> - GCBasher
> - gc-test-suite
> - JPRT
> 
> Please see comments inline.
> 
> > On Mon, 2015-01-26 at 12:02 +0100, Erik Helin wrote:
> > > Hi all,
> > > 
> > > this (rather) small patch removes the field
> > > HeapRegion::_in_collection_set and instead only uses the
> > > G1BiasedMappedArray G1CollectedHeap::_in_cset_fast_test to track
> > > collection set membership. Given that _in_cset_fast_test already track
> > > collection set membership, the _in_collection_set field in HeapRegion is
> > > redundant, it is only messy to keep track of this information in two
> > > places.
> > 
> >  g1CollectorPolicy.cpp:1754: superfluous whitespace after "assert(".
> 
> Fixed.
> 
> >  heapRegion.cpp: in HeapRegion::hr_clear(), is there a reason to remove
> > the assert that checks whether the region is in the collection set or
> > not? I think it could be rewritten the new method easily. (I.e. using
> > the in_collection_set() method in HeapRegion)
> 
> No, you are right, we can keep the assert, I've added it back.
> 
> > I think this change could remove the "_fast_test" suffix of the various
> > methods because now there is only this location where collection set
> > inclusion is tracked. It's just confusing to keep this imo.
> > (and possibly extend "cset" to "collection_set" - this has iirc been
> > done because of the lengthy "fast_set" suffix).
> > 
> > It would be possible to separate that renaming step if you want, but I
> > somehow tend to that that should be part of this change.
> 
> I removed the fast_test sufffix from the
> G1CollectedHeap::register_*_with_in_cset_fast_test methods, but kept
> cset (collection_set becomes too long IMO). I did not remove the suffix
> from the method G1CollectedHeap::clear_cset_fast_test, because the name
> clear_cset is too generic.
> 
> I could also remove the method G1CollectedHeap::in_cset_fast_test, that
> method was apparently unused.
> 
> Thanks,
> Erik
> 
> > Thanks,
> >   Thomas
> > 
> > 
> > 
> >