RFR (L) 8151711: Move G1 number sequences out of the G1 collector policy

Tom Benson tom.benson at oracle.com
Tue Mar 15 19:37:04 UTC 2016


Hi Mikael,
This looks OK to me, with one trivial comment.  Since you're making a 
new module out of it, perhaps you can make the indentation at lines 
105-112 in g1Measurements.cpp more consistent.

If feels like maybe the existing G1Predictions.cpp/.hpp should be folded 
into this new module, now that the measurement stuff is separate from 
Policy.  It's very small - most of the code is a test.  But I suppose 
that could be the subject of another change.

As for names, G1Analytics came to mind to imply data gathering and 
prediction.
Tom

On 3/14/2016 4:25 AM, Mikael Gerdin wrote:
> Hi all,
>
> Currently a large part of the G1 collector policy consists of counters 
> and number sequences for different measurements performed by the 
> collector.
> In order to reduce the overall API surface of the G1CollectorPolicy 
> class and possibly allow for different collector policy 
> implementations the measurements and simple predictions based on the 
> number sequences should be factored out of the policy code. The policy 
> will then become more of a decision maker and not a combined data 
> store and decision maker.
>
> My current working name for the new class is G1Measurements but I'm 
> not overly attached to the name.
>
> I've made the new files "hg copies" of the g1CollectorPolicy files 
> since a lot of methods and members are simply moved as-is to the new 
> class.
> One thing I did modify was to move decisions based on the 
> collector_state() out of the prediction methods and instead based the 
> selection on a boolean parameter.
>
> I suggest that in order to review the changes you open up your 
> favorite 3-way diff tool and view a 3-way side-by-side diff of
> g1CollectorPolicy.cpp (from my webrev),
> g1CollectorPolicy.cpp (from before my suggested changes),
> g1Measurements.cpp (from my webrev).
>
> In diffuse this would be done as:
> diffuse \
> src/share/vm/gc/g1/g1CollectorPolicy.cpp \
> -r qparent src/share/vm/gc/g1/g1CollectorPolicy.cpp \
> src/share/vm/gc/g1/g1Measurements.cpp
>
> This allows you to (hopefully) verify the moved contents.
> I've tried pretty hard to keep the code in the same order as in the 
> original location.
>
> For g1Measurements.hpp I've been a bit more lax to let the header be 
> nice and tidy.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8151711
> Webrev: http://cr.openjdk.java.net/~mgerdin/8151711/webrev.0/
> Testing: RBT gc testing, JPRT, Perf testing on aurora.
>
> Thanks
> /Mikael



More information about the hotspot-gc-dev mailing list