CRR (M): 7097002: G1: remove a lot of unused / redundant code from the G1CollectorPolicy class
tony.printezis at oracle.com
Wed Oct 12 11:22:44 PDT 2011
Thanks to John Cuthbertson for looking at this. Here are the updated
webrevs based on his comments:
On 10/06/2011 04:03 PM, Tony Printezis wrote:
> Hi all,
> This is G1CollectorPolicy cleanup #2, now I got the first one
> (7088680) code reviewed (thanks to Ramki and Bengt). BTW, the webrevs
> below were done on top of the changes for 7088680. So if you want to
> apply them to your workspace you have to apply the patch for 7088680
> first (at least until it's pushed to the GC repo). And the webrevs do
> not include the changes for 7088680 despite what the top-level webrev
> pages say (I checked).
> The motivation for these changes was that there's a lot of unused and
> redundant code in the G1CollectorPolicy class that's been complicating
> some other changes I've been working on (simplify the pause prediction
> model, extend the ergo decision logging, revamp the eden / survivor
> lists, etc.). So, I thought I'd do this cleanup first so that we can
> get it tested separately (as it's not trivial, at least wrt its size)
> and to also keep the code reviews somewhat smaller and simpler.
> The overall webrev is this one:
> Here it is in two parts in case it makes the code reviews easier. BTW,
> I could be persuaded to push each part as a separate changeset if you
> think they are kind of unrelated. That'd be easy to do (I have them as
> two separate MQ patches anyway).
> 1) Remove methods and fields only used by the PREDICTIONS_VERBOSE
> switch and G1PolicyVerbose flag, both of which are also removed.
> We'll replace the functionality of PREDICTIONS_VERBOSE and
> G1PolicyVerbose with extra logging using G1's ergo decision logging
> framework. So, it's good to get rid of all the code that is otherwise
> not used. This is a pure code removal change, I don't believe I added
> or modified anything else.
> 2) Avoid redundant / unnecessary region counting.
> It turns out that we have no less than six (!!!) ways of counting
> regions in the CSet:
> a) YoungList : maintains the count of survivor and eden regions that
> are added to it
> b) _inc_cset_size : the number of young regions added to the
> incremental CSet, which is the same as the total count the YoungList
> c) _collection_set_size : the number of regions added to the CSet,
> which is the same as _inc_cset_size plus any old regions we add to the
> d) _inc_cset_young_index : used to set the young CSet index of the
> young regions (0 for the oldest region, 1 for the second oldest
> region, etc.) which is again essentially the same as _inc_cset_size
> e) _young_cset_length : the number of young regions in the CSet, which
> is already counted by the YoungList
> f) _recorded_young_regions / _recorded_non_young_regions : the number
> of young / old regions in the CSet, again the former is counted by the
> YoungList and the latter is included in _collection_set_size
> I replaced all this by three fields that reflect the number of eden,
> survivor, and old regions in the CSet and a few accessor methods. The
> first two are set using the information the YoungList maintains, the
> third one is incremented every time we add an old region to the CSet.
> The changes are mostly straightforward: remove the unnecessary fields
> and methods and replace them with the new accessor methods. The only
> slightly tricky part was to make sure the young CSet indexes were set
> correctly now that we don't maintain the _inc_cset_young_index (this
> just needed a small refactoring).
> In addition, we maintain counts for the number of mutator allocation
> regions that we allocate. Given that we used to allow such allocations
> to be satisfied out of the old generation, we maintain two counts: one
> for young allocations and one for tenured allocations. It was helpful
> to know what percentage of allocations were satisfied out of the old
> gen. Given that all mutator allocation regions are now allocated out
> of the young gen, this information is irrelevant. So, I also removed
> those fields and associated code.
More information about the hotspot-gc-dev