RFR: 8030646: Track collection set membership in one place

[Erik Helin]

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
> 
> 
> 
>