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

[William Kemper]

On Thu, 21 Dec 2023 01:30:28 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 67:
> 
>> 65: }
>> 66: 
>> 67: void ShenandoahRegulatorThread::regulate_young_and_old_cycles() {
> 
> I am wondering if it might not be simpler and clearer to write this method as:
> 
> 
> ...
> while (!should_terminate()) {
>   switch (mode) {
>      case SCT::none : {
>         // no collection is in progress
>         // 1. check and start a global collection if needed
>         if (should_unload_classes() && start_global_cycle()) {
>           log_info...
>         } else if (start_old_cycle()) {
>            // 2. else check and start an old cycle
>            log_info...
>         } else if (start_young_cycle()) {
>             // 3. else check and start a young cycle
>            log_info...
>         }
>         break;
>      }
>      case SCT::servicing_old: {
>          // if an old collection is in progress, check if a young
>          // collection is needed. We can't start a global collection
>          // in this state, even if one is needed.
>          if (start_young_cycle()) {
>            log_info...
>          }
>          break;
>       }
>       default: // fall through
>    }
>    // sleep a tad before next check; sleep duration inversely proportional to allocation rate
>    regulator_sleep();   // My worry is that heuristics says wait for old increment and we sleep longer than that? May be that's not an issue?
> }
> ...
> 
> 
> The rest of the checks etc. seem already to be done in the `start_...` methods from what I can tell.
> 
> I may have missed some subtlety though.

When regulating young and old cycles (i.e., generational mode), the regulator overrides the global heuristics decision to start a global cycle. In other modes, whether or not to unload classes is decided _after_ a cycle is started. However, the generational mode can only unload classes in a _global_ cycle, so we use the oom in metaspace to _trigger_ a global cycle (even if the global heuristic would not otherwise trigger a cycle).

The other subtlety here is that old cycles are only started when a young cycle wants to run. Each old cycle is preceded by a young "bootstrap" cycle, so the young heuristic trigger is still satisfied. This was meant to avoid the situation where old cycles are triggered shortly after a young cycle - which runs an unproductive "bootstrap" cycle. This is why we check the young heuristic and then the old heuristic before starting an old cycle.

Your comment about starting a global cycle (to unload classes) even during an old cycle is interesting though. There's no specific reason why we couldn't interrupt and abandon the old cycle in this case, but I think that's a change for another PR.

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

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