RFR: 8057586: Explicit GC ignored if GCLocker is active [v3]

[Albert Mingkun Yang]

On Thu, 30 Mar 2023 13:23:09 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2119:
>> 
>>> 2117:     if (counters_before.total_full_collections() != full_gc_count) {
>>> 2118:       return true;
>>> 2119:     }
>> 
>> The other two GCs does this check inside the locker-scope. I don't think there's any practical diff -- why the inconsistency?
>
> Only difference is "time" at which changes were made (Initially on G1), then also I couldn't decide which version is cleaner.

I think the style in Serial is cleaner.

>> src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 556:
>> 
>>> 554:     VMThread::execute(&op);
>>> 555: 
>>> 556:     if (!VM_ParallelGCSystemGC::is_cause_full(cause) || op.full_gc_succeeded()) {
>> 
>> Why `full_gc_succeeded` here? Doesn't the following `full_gc_count != total_full_collections` achieve the same purpose?
>
> `full_gc_count != total_full_collections` is under locker-scope, so we can avoid that locking since we are sure at this point that gc_count was incremented.

True, but this is not a perf-critical method, be consistent (with other gcs) is more important, IMO.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1153276397
PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1153279504