RFR (M) JDK-8151178: Move the collection set out of the G1 collector policy
mikael.gerdin at oracle.com
Mon Mar 7 14:32:33 UTC 2016
On 2016-03-07 15:25, Thomas Schatzl wrote:
> Hi Mikael,
> On Mon, 2016-03-07 at 10:46 +0100, Mikael Gerdin wrote:
>> Hi all,
>> Here's a new version of the webrev based on feedback from Jesper and
>> The incremental layout is similar to version1, with the addition of
>> webrev_4 which contains the review changes.
>> In this update I also took the liberty to change G1CollectionSet to
>> become a value member in G1CollectedHeap instead of a separately C
>> allocated object. I see no value in it being allocated separately.
> some minor issues:
> - maybe the include changes in g1CollectorState.hpp should be a
> separate CR, but then again, it's very small.
I had to change g1CollectorState.hpp, I could not include it otherwise
(unless including allocation.hpp before). I took the liberty to sort it
while I was editing its include list.
> - instead of G1CollectionSet::set_g1p() I would have chosen
> G1CollectionSet::set_policy() for a name.
Hm, yeah I was torn on naming this. On one hand we usually name
accessors after the member variable name. On the other hand I agree that
set_g1p is not a nice name. I also thought about "initialize" or
something, but that makes it sound like it does a lot more.
Maybe I should just rename it _policy an have a set_policy?
> - as a future cleanup suggestion, maybe the CollectionSetChooser could
> be one of the first targets, and probably hidden from direct access by
> the G1CollectorPolicy. Not sure.
It might be possible, but it's not on my refactor list at this time,
> - g1CollectorPolicy.cpp:977 - superfluous newline ;)
Thanks for the review.
> Feel free to implement any of these suggestions, no need for re-review.
More information about the hotspot-gc-dev