RFR (M) JDK-8151178: Move the collection set out of the G1 collector policy (original) (raw)

Tom Benson tom.benson at oracle.com
Mon Mar 7 15:09:25 UTC 2016

Hi Mikael, Looks good to me. I like the idea of changing _g1p to _policy. Tom

On 3/7/2016 9:32 AM, Mikael Gerdin wrote:

Hi Thomas,

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 Tom. http://cr.openjdk.java.net/~mgerdin/8151178/version2/ The incremental layout is similar to version1, with the addition of webrev4 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 -heap 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::setg1p() I would have chosen G1CollectionSet::setpolicy() 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 setg1p 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 setpolicy? - 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, unfortunately. - g1CollectorPolicy.cpp:977 - superfluous newline ;) Will fix. Thanks for the review. /Mikael Feel free to implement any of these suggestions, no need for re-review. Thanks, Thomas

More information about the hotspot-gc-dev mailing list