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

[Tony Printezis]

Bengt,

I have a second cleanup change that's coming up that should remove some 
of the methods you're referring to. Can I do that first? I can open a 
new CR for the cleanup you proposed and extend it to also make sure that 
if-statements always have curly brackets (i.e., a similar change to what 
I did for the CM files).

Tony

Bengt Rutisson wrote:
>
> 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
>