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

[John Coomes]

Tony Printezis (tony.printezis at oracle.com) 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/

I took a quick look and they look fine to me.

-John

> 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