RFR: 8245511: G1 adaptive IHOP does not account for reclamation of humongous objects by young GC

[stefan.johansson at oracle.com]

Hi Ziyi,

On 2020-08-19 19:04, Luo, Ziyi wrote:
> Hi Stefan,
> 
> Thank you for looking into this patch.
> 
> On 8/19/20, 4:10 AM, Stefan Johansson wrote:
> 
>> Hi Ziyi,
>>
>> Thanks for finding this issue and fixing it. The idea for the fix is
>> very easy, take eager reclaim into consideration when deciding the
>> allocation rate for old. Reading the patch and fully understand the
>> calculations was a bit harder. Trying to explain what I would like to
>> see to make it easier to follow is hard. So I've made a proposal that
>> can be applied upon your changes.
>>
>> Inc: http://cr.openjdk.java.net/~sjohanss/8245511/rev1.inc/
>> Full: http://cr.openjdk.java.net/~sjohanss/8245511/rev1.full/
>>
>> Hopefully I did not miss anything that your patch covers that is missing
>> from this proposal. I'll summarize what I did:
>> * Keep track of humongous and non-humongous allocations independently to
>> simplify calculations in reset_after_gc().
>> * Calculate the old generation growth in reset_for_gc(), this avoids
>> saving an additional value and if I'm not missing anything we have all
>> information we need at this point.
> 
> I really don't like the name humongous_bytes_after_penultimate_gc. Thank you
> for helping me get rid of it :)

That name was the trigger for me to look deeper into an alternative 
approach :)

> 
>> * The growth is calculated by adding the non-humongous allocations
>> during the last mutator period with a potential increase in humongous size.
>> * Added _gen to two members to make it clear this is the whole old
>> generation not only old regions.
>>
>> Does these changes all sound good? Let me know what you think.
> 
> I like how you reshaped the code structure especially how you deconstructed
> the net_survived_old_bytes in a clearer fashion. This saves a lot of comments
> to explain the net survived old bytes calculation (now
> last_period_old_gen_growth). The change looks good to me.

Thanks, do you need a sponsor or will you handle that internally at 
Amazon? You can consider this reviewed by me, but we should wait for 
Thomas to take a final look as well.

> 
>>
>> I tried running your repro from the bug-report and it seem to be
>> working, but please try this out and make sure it works in your
>> use-cases as well.
>>
>> Our internal testing looks good so far.
> 
> My local tests look good as well.

Great!

Cheers,
Stefan

> 
> Thanks,
> Ziyi
>