RFR: 8321816: GenShen: Provide a minimum amount of time for an old collection to run [v4]

[William Kemper]

On Thu, 21 Dec 2023 01:34:38 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update stale comment
>
> src/hotspot/share/gc/shenandoah/shenandoahRegulatorThread.cpp line 182:
> 
>> 180: }
>> 181: 
>> 182: bool ShenandoahRegulatorThread::should_unload_classes() {
> 
> I'd rename this to:
> 
> `should_start_metaspace_gc()`
> 
> to avoid confusion with similarly named method in other classes (heuristics) with a different semantics.
> 
> Relatedly, do we still need the second check in this method?
> 
> 
> bool ShenandoahHeuristics::should_unload_classes() {
>   if (!can_unload_classes()) return false;
>   if (has_metaspace_oom()) return true;
>   return ClassUnloadingWithConcurrentMark;
> }
> 
> 
> If not, it can be simplified to `return can_unload_classes() && ClassUnloadingWithConcurrentMark;`
> 
> I do believe, we should write a line documenting the intended semantics of `should_unload_classes()` in all these places because there ay otherwise be some confusion about what the truth modality "should" really means.

I'll add comments to `should_unload_classes`, but I don't want to change the implementation. This method is used by other Shenandoah modes. As I think on it, I'm going to make the regulator honor any class unloading restrictions before starting a fruitless cycle that won't unload anything.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/371#discussion_r1433406832