RFR: 8308507: G1: GClocker induced GCs can starve threads requiring memory leading to OOME [v2]

[Thomas Schatzl]

On Tue, 23 May 2023 09:18:17 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1VMOperations.cpp line 132:
>> 
>>> 130:   // Any allocation requests that were handled during a previous GC safepoint but have not been observed
>>> 131:   // by the requesting mutator thread should be reset to pending. This makes it easier for the current GC to
>>> 132:   // treat the unclaimed memory as garbage.
>> 
>> Suggestion:
>> 
>>   // by the requesting mutator thread should be reset to pending. This makes it easier for the current GC to
>>   // treat the unclaimed memory as garbage. It also simplifies the initial allocation in the safepoint next.
>> 
>> This might cause additional gcs. What would happen if `handle_allocation_requests` just skipped already satisfied allocations (as successful) and only if that fails reset all requests (i.e. around line 148)?
>
> This would create a dependence between handle_allocation_requests, and any collections that happen before. So we would have to deal with how these `fillerObjects` are treated by the collections, some could be invalidated.

In the general case this is true, but here, before GC occurred, the existing allocations can still be considered valid until we know that we need a gc.
Doing so potentially avoids garbage collections because then we are not going to throw away all of them and try to reallocate.

I.e. the suggestion is like:

// keep existing allocations, they are still valid here.

if (has_allocations) {
  if (try_satisfy_allocations()) {  // satisfy non-satisfied allocations.
    return; // no gc occurred, both the previously satisfied allocations and the new allocations are valid.
  }
}
// We are about to do a GC. Reset all allocation requests since we are likely going to free regions containing them.
reset_allocation_requests();

// do gc

instead of the current code

reset_allocation_requests(); // throw away all requests, even already satisfied ones

if (has_allocations) {
  if (try_satisfy_allocations()) { // satisfy all allocations again
    return; // done, all satisfied (no gc occurred)
  }
  reset_allocation_requests(); // throw away all requests, prepare for GC
}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14077#discussion_r1201980307