RFR 7097567: G1: abstract and encapsulate collector phases and transitions between them

[Derek White]

Hi Mikael,

Thanks for the review. I just got back to this. Comments inline, and new 
webrev below...

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.
OK
> 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.
OK
> The change to compactibleFreeListSpace seems like an accident since it 
> has nothing to do with G1 state changes whatsoever.
OK

- Derek

---------- NEW WEBREV ----------

Bug: https://bugs.openjdk.java.net/browse/JDK-7097567
Webrev: http://cr.openjdk.java.net/~drwhite/7097567/webrev.02/

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...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150512/d18033d4/attachment.htm>