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

[Mikael Gerdin]

On 2015-11-05 13:40, Thomas Schatzl wrote:
> Hi Mikael,
>
> On Thu, 2015-11-05 at 13:10 +0100, Mikael Gerdin wrote:
>> Hi Thomas,
>>
>> On 2015-11-05 11:49, Thomas Schatzl wrote:
>>> Hi all,
> [...]
>>> 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?
>>
>
> Yes. A value of "1" is no safety at all. I plan to look at the default
> value of G1ConfidenceFactor, because 0.5 seems "pretty uncertain" to me
> already as that value is located halfway the possible range.
> I would be fine to drop the G1ConfidenceFactor term though if somebody
> objects.
>
> I am pretty certain that the TargetPLABWastePct is a good idea - that is
> after all our goal in how much we can waste for PLAB sizing. And GC
> threads typically allocate in PLABs from the heap, not in actual
> objects.

I agree that the PLAB waste factor should be present.

>
> Also consider that this check is applied for every single region, so
> e.g. it's not as if we want to evacuate 100 half-full (1M) regions (=50M
> data), this value of 2.2 does not means that we sort of keep >50M as
> reserve, only around 1M (with these settings).

Right, that's my understanding as well.

Ship it!

/Mikael

>
>>> 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
>
>