CRR (M): 7145441: G1: collection set chooser-related cleanup
Tony Printezis
tony.printezis at oracle.com
Fri Apr 13 18:20:36 UTC 2012
Hi all,
I'd like a couple of code reviews for the cleanup of the collection set
chooser class. This is the whole webrev (BTW, it depends on the size_t
-> uint changes that I will push soon):
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.all/
I broke down the change into the following five (a personal record I
think!) parts:
0. Remove a lot of unused code that was made redundant after the recent
changes to the CSet chooser. I also removed the G1PrintParCleanupStats
flag which we have not used for a while.
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.0.G1CSetChooserCleanup0/
1. The naming style of methods / fields of the CSet chooser class was
quite messy. I changed any names in ThisStyle to this_style. I also
simplified some names which tended to be quite verbose and/or inaccurate
(getParMarkedHeapRegionChunk() -> claim_array_chunk(),
addMarkedHeapRegion() -> add_region(), etc.). Generally, not point in
repeating "marked regions" given this is the only type of regions added
to the CSet chooser.
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.1.G1CSetChooserCleanup1/
2. Standardized the type for region nums / indexes to uint to be
consistent with the rest of G1 (we had a mix of ints / jints). The
problem with is is that we use a GrowableArray to keep track of all the
regions added to the CSet chooser which uses ints for indexes and
lengths. To avoid excessive casting I created wrappers that cast uints
to ints and vice versa (couldn't come up with a more elegant solution).
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.2.G1CSetChooserCleanup2/
3. I removed the sort_index field and method from the HeapRegion class,
as it's not really needed any more. I also removed the
assertMarkedBytesDataOK() method and, where appropriate, I call
CSetChooser::verify() instead which does similar checks.
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.3.G1CSetChooserCleanup3/
4. In the end, there was still some redundant code left: the total_live,
total_used, etc. fields on the CalcLiveObjectsClosure and
FinalCountDataUpdateClosure closures (they were used only in the
G1PrintParCleanupStats output) and the known garbage, GC efficiency,
etc. fields on the G1 policy (they were used in the previous
implementation of "when to do mixed GCs" policy), etc. I removed all
that too.
http://cr.openjdk.java.net/~tonyp/7145441/webrev.0/webrev.4.G1CSetChooserCleanup4/
Looking at the overall webrev, it's not too bad to review so I'd start
from that one. However, I did maintain the changes on the separate
patches for various reasons so I thought I'd post those here anyway.
Tony
More information about the hotspot-gc-dev
mailing list