RFR: 8030646: Track collection set membership in one place

Thomas Schatzl thomas.schatzl at oracle.com
Tue Jan 27 09:32:20 UTC 2015


Hi,

  thanks for taking this over.

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(".

 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)

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.

Thanks,
  Thomas






More information about the hotspot-gc-dev mailing list