RFR: 8355681: G1HeapRegionManager::find_contiguous_allow_expand ignores free regions when checking regions available for allocation

[Albert Mingkun Yang]

On Mon, 28 Apr 2025 10:57:48 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

> Hi,
> 
> Please review this change to account for free regions when checking if we have enough regions to satisfy an allocation request. Currently, we have that a `_hrm.expand_and_allocate_humongous` call fails where an `expand_and_allocate` call succeeds for the same allocation request. 
> 
> Testing: Tier 1-3

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 909:

> 907:     // For humongous objects, we should have expanded the heap on the first
> 908:     // attempt_allocation_at_safepoint above.
> 909:     result = expand_and_allocate(word_size);

Why `attempt_allocation_at_safepoint` performs expansion for humongous objs but not ordinary objs? Since `attempt_allocation_at_safepoint` doesn't contain "expand" in its name, I'd expect it not to perform expansion at all. (If expansion for humongous objs is critical, I wonder if it makes sense to branch on `is_humongous` at the beginning of this method and handle those two cases in two diff paths.)

src/hotspot/share/gc/g1/g1HeapRegionManager.cpp line 481:

> 479: uint G1HeapRegionManager::find_contiguous_allow_expand(uint num_regions) {
> 480:   // Check if we can actually satisfy the allocation.
> 481:   if (num_regions > (num_free_regions() + available())) {

The name "available" is too vague -- without looking at its impl, one could think free regions should be "available" as well. I wonder if sth like "num_uncommitted" is more precise.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24915#discussion_r2068400093
PR Review Comment: https://git.openjdk.org/jdk/pull/24915#discussion_r2068393125