RFR (XS): 8138684: G1 decision about taking regions into the collection set is too aggressive

[Mikael Gerdin]

Hi Thomas,

On 2015-11-05 11:49, Thomas Schatzl wrote:
> Hi all,
>
>    can I have reviews for this small change to the policy that ends
> adding regions to the collection set?
>
> In some earlier review we noticed that G1 tries to fill up all remaining
> space completely.
>
> E.g. in
>
> 466 if ((2.0 * _sigma) * (double) bytes_to_copy > (double) free_bytes)
> {
> 467 // end condition 3: out-of-space (conservatively!)
> 468 }
>
> _sigma is .5 by default, and I have never seen anyone changing
> G1ConfidencePercent.
>
> This is a recipe for evacuation failure we want to avoid.
>
> So I thought of a formula that better represents the assumptions in the
> GC code, and came up with this:
>
>   487   double safety_factor = (100.0 / G1ConfidencePercent) * (100 +
> TargetPLABWastePct) / 100.0;
>   488   size_t expected_bytes_to_copy = safety_factor * bytes_to_copy;
>   489
>   490   if (expected_bytes_to_copy > free_bytes) {
>
> I.e. the safety factor is first dependent on our target PLAB waste, and
> on the confidence of our predictions in general, scaling inversely (i.e.
> with high G1ConfidencePercent we assume that our assumption about
> keeping TargetPLABWastePct is more correct, and with lower
> G1ConfidencePercent we allow more slack).
>
> A (really) conservative buffer here is particularly interesting for
> dynamic IHOP, as it tends to try to push out marking rounds as long as
> possible.

Just so I'm reading this code correctly, we are essentialy changing from 
a "safety factor" of 1 to a safety factor of 2,x where x depends on the 
PLAB waste percent?



>
> Since we are also in the process of investigating the policy surrounding
> mixed gcs and the predictions, this seems a lot better starting point
> too.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8138684
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8138684/webrev
> Testing:
> jprt, benchmark runs with and without dIHOP without any real
> differences.

Given that you've performance tested this without any issues I think 
this is good to go.

/Mikael


>
> Thanks,
>    Thomas
>
>