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

[Albert Mingkun Yang]

On Mon, 19 May 2025 14:33:12 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - review
>>  - Revert "review"
>>    
>>    This reverts commit 666cc5a1cbc72453a09fdf4e9319bf140365859e.
>
> 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.

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

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