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

[Bengt Rutisson]

Looks good to me.

One question:

In several places you have made sure that the method declarations are on 
a single line (not having the return type on a separate line above). I 
like this, but there are still lots of methods that have their return 
type on a separate line. Since this is a cleanup CR I would suggest 
changing all method declarations to be consistent.

Bengt

On 2011-10-04 23:01, 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