RFR: 8325670: GenShen: Allow old to expand at end of each GC

[Kelvin Nilsen]

On Tue, 20 Feb 2024 00:46:04 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> I am beginning to better understand what you were trying to achieve, but I am still not quite there.
>> 
>> Is there a natural sensible limit at which `max_old_reserve` can be bounded? It would seem then that, since you were not previously bounding the computation of `max_old_reserve` in any manner and you don't want to bound it to `old_available + xfer_limit`, that a more natural and essentially largest possible value would be the sum of what young can promote and what old can evacuate, which would look something like `heap->max_capacity()`, since it would effectively be morally equivalent to imposing no limits on `max_old_reserve`.
>> 
>> Alternatively, if you are considering changing this whole thing anyway, perhaps we just do that directly. If you expect that PR to take a while and you just want to restore old behaviour, I'd suggest bounding the calculation of `max_old_reserve` to `heap->max_capacity()`, since that is a natural limit irrespective of what SOERP happens to be (and not artificial and confusing like the one that you suggested covering that one case of SOERP sending the value to NaN but not otherwise bounding it and allowing it to grow arbitrarily large and wrapping around).
>> 
>> In other words, I suggest using:
>> 
>>   const size_t max_old_reserve = (ShenandoahOldEvacRatioPercent == 100) ?
>>     heap->max_capacity() : MIN2((young_reserve * ShenandoahOldEvacRatioPercent) / (100 - ShenandoahOldEvacRatioPercent),
>>                                 heap->max_capacity());
>> 
>> 
>> Let me know if that makes sense.
>
>> ... covering that one case of SOERP sending the value to NaN but not otherwise bounding it and allowing it to grow arbitrarily large and wrapping around).
> 
> I realize that there is in fact a natural bound to that value when SOERP < 100, viz. when it's 99 (since it's not a float): `young_reserve * 99/(100-99)`, i.e. `99 * young_reserve`. I guess the simpler thing to do then is to just avoid this completely and declare `ShenandoahEvactReserve` to `range(1,99)` and be done, throwing awy the protection for the lone case of `SOERP=100` -- after all we don't allow `SOERP=0`, so by symmetry it looks like we shouldn't allow 100 either, just `range(1,99)`.

Thanks for these suggestions.    I'll see if I can stabilize a solution that works for all cases.

-------------

PR Review Comment: https://git.openjdk.org/shenandoah/pull/394#discussion_r1495851540