RFR: 8317755: G1: Periodic GC interval should test for the last whole heap GC [v3]

[Y. Srinivas Ramakrishna]

On Tue, 28 Nov 2023 17:51:31 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> See the description in the bug. Fortunately, we already track the last whole-heap GC. The new regression test verifies the behavior.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug `tier1 tier2 tier3`
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8317755-g1-periodic-whole
>  - Merge branch 'master' into JDK-8317755-g1-periodic-whole
>  - Optional flag
>  - Merge branch 'master' into JDK-8317755-g1-periodic-whole
>  - Keep the cast
>  - Fix

Hi Thomas @tschatzl :

> > LGTM. Very useful when there is no direct feedback loop from resource pressure that we might otherwise have used for signaling the need for a whole heap GC, so is a good escape hatch to put in the hands of service owners. Of course, like any escape hatch, it could be misused, but I don't think that is a concern about ejection seats from fighter jets for example. We trust those who fly the jets not to eject for no reason.
> > I would suggest formally dissociating this from the JEP itself. The fix ups to the JEP's and its implementation shortcomings can be addressed fully in the fullness of time. My USD 0.02.
> 
> I do not think breaking existing functionality to add new functionality given that the former is widely publicized via the JEP for the default collector and "fixing it up later" is the proper way forward. The only advantage I can see with this approach is that this is a simple change, but implementing the new functionality properly is likely not that much more work anyway.

Thanks for the response.

I read the JEP (authored by you, Thomas). It got the impression from the verbage there (albeit slightly ambiguous) that the intention was to do the periodic gc's which would induce a concurrent major cycle and collect the entire heap, because the objective was to run a cycle that reclaims garbage in the absence of other triggers and returns memory to the OS. The JEP also states that a major cycle is essential because a minor cycle doesn't suffice. As such, the only rational solution that meets the proposed objectives of the JEP is to consider major cycles as the criterion for the timer whose tripping triggers the major cycle.

I submit that, from this point of view, this PR actually corrects a small defect in the original implementation, and does so in a backward compatible fashion, so that any users dependent on the old, arguably defective, behavior are not surprised.

I think your stance, if I understood correctly, is that there are other things that we have learned since that need to be corrected too, but it looks like we are then allowing the perfect to be the enemy of the better, in allowing this correction to be made.

I may have misunderstood the long comment stream, so will go back and reread in case I missed something crucial about the objection to this PR in its current form.

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

PR Comment: https://git.openjdk.org/jdk/pull/16107#issuecomment-1969651859