RFR: 8357218: G1: Remove loop in G1CollectedHeap::try_collect_fullgc [v4]

[Thomas Schatzl]

On Tue, 20 May 2025 05:05:15 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 1885:
>> 
>>> 1883:                         cause);
>>> 1884:     VMThread::execute(&op);
>>> 1885:     return op.prologue_succeeded();
>> 
>> They might now be the same value, but `gc_succeeded()` seems like a better semantic fit here.
>> Looking around a little, it's not obvious why `prologue_succeeded()` is ever the right name for a
>> client-accessible test for what seems like gc-completion.  But that's a separate issue.
>
> How about introducing a local var to better reflect the semantics?
> 
> 
> // GCs always run-to-completion once prologue succeeds.
> bool is_gc_succeeded = op.prologue_succeeded();
> return is_gc_succeeded;
> 
> 
> This way we don't need to maintain a "redundant" field in `VM_G1CollectFull`.
> 
> (Thomas also asked whether the same field in `VM_G1CollectForAllocation` is actually needed -- seems that `_gc_succeeded == prologue_succeeded()` holds as well, so we can potential remove that one as well.)
> 
> If you dislike the suggestion, I will add back the removed field.

The field does not need to be re-added in the full gc VM op, just add an accessor `gc_succeeded()` that returns the internal `prologue_succeeded()`.

I will file a bug for removing the `_gc_succeeded` field in `VM_G1CollectForAllocation`, because it is always true if `doit` ran.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25296#discussion_r2097142894