CRR (Small-ish): 7088680: G1: Cleanup in the G1CollectorPolicy class

[Tony Printezis]

I also meant to add to the CRR text: While reviewing this change please 
consider the following. Before a Full GC we tear down most region lists 
/ sets, but leave the humongous set as is (as we currently don't move 
humongous regions during the Full GC). During the Full GC, any humongous 
regions we find empty we have to remove from the humongous set. After 
the Full GC, while we recreate the other region lists / sets, we don't 
have to recreate the humongous set, as it's already correctly formed. I 
think the code might be a bit more straightforward if we also tear-down 
/ recreate the humongous region set along with the rest, especially 
given that we might want to move some humongous regions in the future 
during a Full GC. Let me know if you think this will be a worthwhile 
change (we can do this as a separate CR).

On 10/04/2011 05:01 PM, Tony Printezis wrote:
> Thanks Mikael,
>
> Yes, I accidentally posted the wrong links. The correct ones are:
>
> all) http://cr.openjdk.java.net/~tonyp/7088680/webrev.0/webrev.all/
>
> 1) 
> http://cr.openjdk.java.net/~tonyp/7088680/webrev.0/webrev.0.G1G1PCleanupFieldMethodRemoval/
>
> 2) 
> http://cr.openjdk.java.net/~tonyp/7088680/webrev.0/webrev.1.G1G1PCleanupMethodRefactoring/
>
> 3) 
> http://cr.openjdk.java.net/~tonyp/7088680/webrev.0/webrev.2.G1G1PCleanupDevirtualize/
>
> Apologies!
>
> Tony
>
> On 10/4/2011 4:14 PM, Mikael Gerdin wrote:
>> Maybe you should upload your webrevs to cr.openjdk.java.net :)
>> /mgerdin
>>
>> On Tuesday 04 October 2011 14.57.10 Tony Printezis wrote:
>>> Hi all,
>>>
>>> I'd like to get a couple of code reviews for some cleanup in the
>>> G1CollectorPolicy class. The changes were motivated by code reviewers'
>>> comments on a previous CR (thanks John). The webrev for all the changes
>>> is here:
>>>
>>> http://syros.us.oracle.com/local_ws/hs-g1-g1p-cleanup/7088680/webrev.0/webre 
>>>
>>> v.all/
>>>
>>> I have broken the changes down in three different webrevs in case they
>>> are a little easier to code review this way:
>>>
>>> 1) Remove unnecessary fields and methods:
>>>
>>> http://syros.us.oracle.com/local_ws/hs-g1-g1p-cleanup/7088680/webrev.0/webre 
>>>
>>> v.0.G1G1PCleanupFieldMethodRemoval/
>>>
>>> 2) Remove the G1CollectoryPolicy_BestRegionsFirst class and fold its
>>> functionality in its superclass, i.e., G1CollectorPolicy. Currently, we
>>> only have one policy so there's no point in having the separation. And
>>> the separation was done in a very ad-hoc way anyway. If we want to have
>>> more than one policy in the future we should really rethink on how 
>>> it is
>>> done. Most of the changes were straightforward, the slightly more
>>> involved one was to combine various
>>> record_concurrent_mark_cleanup_end*() methods into one and rename some
>>> of the variables to make sure their names are consistent. I also 
>>> changed
>>> protected fields into private, given that protected fields again do not
>>> make sense any more.
>>>
>>> http://syros.us.oracle.com/local_ws/hs-g1-g1p-cleanup/7088680/webrev.0/webre 
>>>
>>> v.1.G1G1PCleanupMethodRefactoring/
>>>
>>> 3) Make several methods in G1CollectorPolicy non-virtual as it makes no
>>> sense for them to be virtual:
>>>
>>> http://syros.us.oracle.com/local_ws/hs-g1-g1p-cleanup/7088680/webrev.0/webre 
>>>
>>> v.2.G1G1PCleanupDevirtualize/
>>>
>>> Thanks,
>>>
>>> Tony