RFR: 8138975: G1CollectorPolicy::calculate_young_list_target_length should be const

[Thomas Schatzl]

Hi Erik,

On Mon, 2015-10-19 at 17:01 +0200, Erik Helin wrote:
> On 2015-10-15, Thomas Schatzl wrote:
> > Hi Erik,
> > 
> > On Wed, 2015-10-07 at 14:16 +0200, Erik Helin wrote:
> > > On 2015-10-07, Mikael Gerdin wrote:
> > > > Hi Erik,
> > > > 
> > > > On 2015-10-06 17:28, Erik Helin wrote:
> > [...]
> > > > >
> > > > >Enhancement:
> > > > >https://bugs.openjdk.java.net/browse/JDK-8138975
> > > > >
> > > > >Webrev:
> > > > >http://cr.openjdk.java.net/~ehelin/8138975/webrev/
> > > > 
> > > > I discussed the is_young bool parameter with Erik and we settled on removing
> > > > it at this point.
> > > > A lot of the underlying code still looks at the collector_state instead of
> > > > getting young/mixed via a parameter so we'll leave that as is for now.
> > > 
> > > Yep, I've removed the is_young parameter to
> > > bounded_young_list_target_length. Please see new webrevs:
> > > 
> > > Full:
> > > http://cr.openjdk.java.net/~ehelin/8138975/webrev.01/
> > > 
> > > Incremental:
> > > http://cr.openjdk.java.net/~ehelin/8138975/webrev.00-01/
> > 
> >   the change does not apply to recent tip any more; I tried to fix them
> > to do a preliminary review. Looks good, with one minor comment: it's
> > rs_lengths, not rs_lenghts :)
> 
> I fixed the typo, thanks for catching that :)
> 
> > In any case I would like to have another look at a properly merged
> > version.
> 
> Please see new fully rebased patches at:
> 
> - Full:
> http://cr.openjdk.java.net/~ehelin/8138975/webrev.02/
> 
> - Incremental (only two typos corrected):
> http://cr.openjdk.java.net/~ehelin/8138975/webrev.01-02/
> 

  some pre-existing issue:

- G1CollectoryPolicy::init(), line 434, the call to
update_young_list_target_length() should be a call to
update_young_list_target_length(0).

At the start we know that rs_length is zero, there is no need to get a
prediction from an empty sequence.

Feel free to ignore, as that and related issues have already been filed
under JDK-8139594.

I mention this, because adding this would remove the need for the
overload.

Looks good otherwise. I do not need a re-review if you decide to change
this as suggested.

Thanks,
  Thomas