RFR (M): 8137082: Factor out G1 prediction code from G1CollectorPolicy and clean up

[Thomas Schatzl]

Hi,

On Wed, 2015-10-14 at 14:54 -0400, Kim Barrett wrote:
> On Oct 14, 2015, at 11:44 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> > 
> > On Tue, 2015-10-13 at 18:57 -0400, Kim Barrett wrote:
> >> On Oct 13, 2015, at 8:26 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
[...]
> > New webrev at
> > 
> > http://cr.openjdk.java.net/~tschatzl/8137082/webrev.3/ (full)
> > http://cr.openjdk.java.net/~tschatzl/8137082/webrev.2_to_3/ (diff) 
> 
> Looks good, other than a couple minor issues.  I don't think I need a
> new webrev for any of these.
> 
> ------------------------------------------------------------------------------  
> src/share/vm/gc/g1/g1CollectorPolicy.cpp 
>  155   // SurvRateGroups below must be initialized after '_sigma' because they
>  156   // indirectly access '_sigma' through this object passed to their constructor.
> 
> Pre-existing.  Maybe I'm wrong, but I don't see any sign of this being
> true either before or after this set of changes.  Maybe there have
> been other changes since that comment was added 8 months ago (by me!)
> that made it no longer so?

This comment is actually correct. Through a pretty long call-chain,
SurvRateGroup calls get_new_prediction() in
SurvRateGroup::all_surviving_words_recorded, which uses the predictor's
sigma of course.

I adapted the comment though.

> ------------------------------------------------------------------------------  
> src/share/vm/gc/g1/g1CollectorPolicy.hpp 
> 
> Are there any remaining uses of G1CollectorPolicy::sigma or _sigma?
> It looks like these might be dead after your other changes.  And if
> sigma() *is* still needed, it probably should be obtained from the
> _predictor rather than duplicating information there.

It can be removed, it's dead code. Thanks for catching this.

> 
> ------------------------------------------------------------------------------ 
> src/share/vm/gc/g1/g1Predictions.hpp
>   52     assert(sigma >= 0.0, "Confidence must be larger than zero");
> 
> "larger than zero" => "at least zero" or "non-negative" or something.
> 
> ------------------------------------------------------------------------------
> 

Fixed. Thanks.

For reference, I put the "final" webrevs here:

http://cr.openjdk.java.net/~tschatzl/8137082/webrev.4/ (full)
http://cr.openjdk.java.net/~tschatzl/8137082/webrev.3_to_4/ (diff) 

Thanks,
  Thomas