RFR 7097567: G1: abstract and encapsulate collector phases and transitions between them
derek.white at oracle.com
Tue May 12 21:26:24 UTC 2015
Thanks for the review. I just got back to this. Comments inline, and new
On 1/28/15 5:03 AM, Mikael Gerdin wrote:
> Hi Derek,
> On 2015-01-27 23:41, Derek White wrote:
>> Review request!
>> This is a change Ramki did before he left that didn't get checked in.
>> The basic idea is to move a bunch fields that encapsulate collector
>> state from G1CollectorPolicy into a separate class " G1CollectorState".
>> Thanks in advance!
>> - Derek
>> /Bug/: https://bugs.openjdk.java.net/browse/JDK-7097567
>> /Webrev/: http://cr.openjdk.java.net/~drwhite/7097567/webrev.00/
> This is just a high level review to begin with, I didn't look too
> carefully at all the state changes.
> I agree that it's a good idea to factor out the state changes but I'm
> not convinced that the CollectorState should logically be a member of
> the collector policy object.
> To me it feels more like a G1 heap would have a policy object and a
> state object as members.
I talked this over with Thomas and now understand the issue. I also
sucked up some state fields from g1CollectedHeap into g1CollectorState.
> If the policy object needs to interact with the state object they can
> of course do that via a pointer to the state object from the policy.
> Besides that I don't like the fact that the state object is declared
> in the g1CollectorPolicy.hpp header, I'd prefer it if you could move
> that to a separate new header file.
> The change to compactibleFreeListSpace seems like an accident since it
> has nothing to do with G1 state changes whatsoever.
---------- NEW WEBREV ----------
I also added follow-on enhancement request for turning G1CollectorState
into a real state machine.
* JDK-8080226 <https://bugs.openjdk.java.net/browse/JDK-8080226> G1:
Replace collector state booleans with explicit state variable(s)
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev