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