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

[Eric Caspole]

Hi Thomas,
This looks good to me, I like the "stand alone" prediction class.
Eric


On 10/13/2015 08:26 AM, Thomas Schatzl wrote:
> Hi all,
>
>    due to recent changes (mostly Erik's latest refactoring to the
> G1CollectorPolicy already doing a lot of const'ification) I had to
> rebase this change on the latest hotspot.
>
> The webrevs include only rebase changes, no new changes.
>
> http://cr.openjdk.java.net/~tschatzl/8137082/webrev.2/ (full)
> http://cr.openjdk.java.net/~tschatzl/8137082/webrev.0_to_2/ (diff)
>
> Thanks,
>    Thomas
>
> On Thu, 2015-10-01 at 17:32 +0200, Thomas Schatzl wrote:
>> Hi Jon,
>>
>> On Thu, 2015-10-01 at 08:25 -0700, Jon Masamitsu wrote:
>>> Thomas,
>>>
>>> Looks good.  Thanks for the changes.
>>>
>>> In g1Predictions.cpp you set epsilon to 1e-6.
>>> Is that value based on what you've seen as
>>> being needed to avoid floating point inaccuracies?
>>> Or just a nice conservative number that should never
>>> fail?
>>>
>>
>> The 1e-6 is just some typical "small" number to counter floating point
>> representation problems during comparison.
>> It has nothing to do with the prediction at all.
>>
>> Let me see if I can find something like that in the existing code.
>>
>>> If you decide to add a comment about what this code
>>> is trying to do (as I requested in my last mail)
>>>
>>>    469   if ((2.0 /* magic */ * _predictor.sigma()) * (double) bytes_to_copy > (double) free_bytes) {
>>>
>>> I don't need a new webrev.
>>
>> I added comments to JDK-8138684. I expect that to be fixed soon, the
>> current code makes G1 take too much risk in adding regions, potentially
>> causing evacuation failures. This degrades the G1 experience :)
>>
>> Thanks,
>>    Thomas
>>
>>
>
>