RFR: 8369111: G1: Determining concurrent start uses inconsistent predicates

Ivan Walulya iwalulya at openjdk.org
Fri Oct 31 15:32:31 UTC 2025


On Tue, 14 Oct 2025 08:58:43 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   please review this change that fixes an inconsistency between requesting a concurrent start garbage collection during humongous object allocation and then actually starting it.
> 
> I.e. in `G1CollectedHeap::attempt_allocation_humongous` we check whether the allocation would cross the IHOP threshold taking the current allocation into account, and if so, see if G1 should start a concurrent marking, eventually starting a GC pause.
> 
> That GC pause did not take the prospective allocation into account, so we could do that GC for nothing (i.e. not start a concurrent marking although we already knew that the allocation would cause one).
> 
> This, in conjunction with JDK-8368959 can cause hundreds of extra GCs for the test in the CR (without eager reclaim of humongous arrays with references); otherwise it could cause the marking starting too late.
> 
> There is a second bug in the calculation whether G1 crossed the threshold: for humongous objects it only takes the actual size into account, not the size that is needed for allocating it. The same issue existed for determining to start a concurrent mark after any other collection too.
> 
> The change also tries to unify naming of the parameter to pass the allocation size (`alloc_word_size` -> `allocation_word_size`) and the parameter order where this size is passed along in multiple related methods.
> 
> Testing: mentioned test case now behaving correctly, tier1-5
> 
> Thanks,
>   Thomas

LGTM!

Nit:

src/hotspot/share/gc/g1/g1VMOperations.cpp line 56:

> 54:                              _gc_cause == GCCause::_wb_full_gc;
> 55:   g1h->do_full_collection(size_t(0) /* allocation_word_size */,
> 56:                           clear_all_soft_refs /* clear_all_soft_refs */,

don't see the value added by the comment for the named variable `clear_all_soft_refs`

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

Marked as reviewed by iwalulya (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27789#pullrequestreview-3404793199
PR Review Comment: https://git.openjdk.org/jdk/pull/27789#discussion_r2481785749


More information about the hotspot-gc-dev mailing list