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

[Mikael Gerdin]

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.

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.

/Mikael

> /Ran/:
>
>   * jtreg
>   * jprt
>   * refworkload (looks good, but improvement is unexplained).
>
>     ./compare -r Logs/ref.1/ Logs/cms.1/
>     ==============================================================================
>     Logs/ref.1/:
>        Benchmark           Samples        Mean Stdev             Geomean
>     Weight
>        specjbb2000              15   660639.27 8546.73
>          First_Warehouse        15    91712.47 2908.19
>          Last_Warehouse         15   660639.27 8546.73
>          rw.runtime             15      604.00 0.00
>        specjbb2005              15   372212.33 8495.85
>          first                  15    44054.38 2190.33
>          interval_average       15     7596.13 173.44
>          last                   15   372212.33 8495.85
>          last_warehouse         15        8.00 0.00
>          overall_average        15   342311.21 27049.43
>          peak                   15   418062.71 21056.23
>          peak_warehouse         15        2.87 1.25
>          rw.runtime             15      458.40 0.99
>        specjvm98                15      950.96 10.07
>          compress               15      695.97 7.54
>          db                     15      361.98 6.56
>          jack                   15     1461.28 51.26
>          javac                  15      522.44 13.72
>          jess                   15     1145.71 15.97
>          mpegaudio              15     1197.56 14.91
>          mtrt                   15     2670.94 137.46
>          rw.runtime             15       36.07 0.59
>     ==============================================================================
>     Logs/cms.1/:
>        Benchmark           Samples        Mean     Stdev %Diff     P
>     Significant
>        specjbb2000              15   646579.89   5829.25 -2.13
>     0.000          Yes
>          First_Warehouse        15    86084.97   1381.09 -6.14
>     0.000          Yes
>          Last_Warehouse         15   646579.89   5829.25 -2.13
>     0.000          Yes
>          rw.runtime             15      604.33      0.49 -0.06
>     0.019            *
>        specjbb2005              15   372333.48   8449.27 0.03
>     0.969            *
>          first                  15    44028.81    591.86 -0.06
>     0.966            *
>          interval_average       15     7598.67    172.40 0.03
>     0.968            *
>          last                   15   372333.48   8449.27 0.03
>     0.969            *
>          last_warehouse         15        8.00      0.00 0.00
>     1.000            *
>          overall_average        15   347967.16   8974.17 1.65
>     0.453            *
>          peak                   15   427459.90  13498.58 2.25
>     0.159            *
>          peak_warehouse         15        2.27      0.46 20.93
>     0.097            *
>          rw.runtime             15      458.47      0.52 -0.01
>     0.819            *
>        specjvm98                15      891.61      9.95 -6.24
>     0.000          Yes
>          compress               15      696.34      6.57 0.05
>     0.888            *
>          db                     15      363.90      5.55 0.53
>     0.395            *
>          jack                   15     1402.76     48.98 -4.01
>     0.003          Yes
>          javac                  15      453.21      8.57 -13.25
>     0.000          Yes
>          jess                   15     1024.87      7.81 -10.55
>     0.000          Yes
>          mpegaudio              15     1146.54     37.59 -4.26
>     0.000          Yes
>          mtrt                   15     2369.88     70.39 -11.27
>     0.000          Yes
>          rw.runtime             15       38.00      0.38 -5.36
>     0.000          Yes
>     ==============================================================================
>        * - Not Significant: A non-zero %Diff for the mean could be
>     noise. If the
>            %Diff is 0, an actual difference may still exist. In either
>     case, more
>            samples would be needed to detect an actual difference in
>     sample means.
>            Alpha for this run: 0.010
>
>