RFR: 8357218: G1: Remove loop in G1CollectedHeap::try_collect_fullgc
Thomas Schatzl
tschatzl at openjdk.org
Mon May 19 12:02:54 UTC 2025
On Mon, 19 May 2025 08:25:02 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 1875:
>>
>>> 1873: // Request is trivially finished.
>>> 1874: if (!GCCause::is_explicit_full_gc(cause) || op.gc_succeeded()) {
>>> 1875: return op.gc_succeeded();
>>
>> Complete removal of this part isn't correct. The premise of this change, that "full-gc always
>> run-to-completion" is not correct. A `_g1_periodic_collection` may be cancelled:
>> https://github.com/openjdk/jdk/blob/50a7c61d28b9885ff48f4fcd8bfd460b507bbcef/src/hotspot/share/gc/g1/g1VMOperations.cpp#L39-L48
>> Such a collection is not an explicit full-gc, so that part of the test may be true, when this function
>> returns false because the gc did not succeed.
>
> Updated the description. The overall result of `!GCCause::is_explicit_full_gc(cause) || op.gc_succeeded()` is always true.
The pre-existing code seems to be buggy: if the operation had been skipped, it should have returned false given the one caller that reads the boolean result (periodic collection invocation) to print a log line. I.e. maybe something like:
`return op.prologue_succeeded() && op.gc_succeeded();`
Similar to `G1CollectedHeap::do_collection_pause()`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25296#discussion_r2095521025
More information about the hotspot-gc-dev
mailing list