RFR: 8271884: G1CH::_expand_heap_after_alloc_failure is no longer needed

[Thomas Schatzl]

On Tue, 10 Aug 2021 20:24:42 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi,
>> 
>> Please review this change to remove _expand_heap_after_alloc_failure. Checks for old_is_full(), survivor_is_full(), and G1CollectedHeap::has_more_regions() should eliminate repeated expansion attempts.
>> 
>> Testing; tier 1
>
> src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 167:
> 
>> 165:   HeapRegion* res = _hrm.allocate_free_region(type, node_index);
>> 166: 
>> 167:   if (res == NULL && do_expand) {
> 
> Ivan and me were looking into any adverse impact when removing this, in particular the expansion being called very often without `_expand_heap_after_alloc_failure`.
> There is indeed an issue with convoying in both the survivor and old gen region allocation; the code that ultimately calls down here looks like the following:
> 
>   if (result == NULL && !old_is_full()) {
>     MutexLocker x(FreeList_lock, Mutex::_no_safepoint_check_flag);
>     result = old_gc_alloc_region()->attempt_allocation_locked(min_word_size,
>                                                               desired_word_size,
>                                                               actual_word_size);
>     if (result == NULL) {
>       set_old_full();
>     }
>   }
>   ```
> (likewise for survivor allocation)
> After the first check to `old_is_full()` we get the `FreeList_lock` and after that call down to the allocation. This is actually the problem: multiple threads can be waiting for the lock trying to allocate, and all of these waiters will be able to try to expand without that `_expand_heap_after_alloc_failure` flag.
> The solution is to check `old_is_full()` after grabbing the lock again - which is actually better than only finding out here quite late that there is nothing to allocate.

We should probably check if there is a similar convoying effect for mutator allocation, possibly in a separate CR.

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

PR: https://git.openjdk.java.net/jdk/pull/5030