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

[Albert Mingkun Yang]

On Tue, 28 Mar 2023 18:30:19 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi All,
>> 
>> Please review this change to guarantee that at least a Full GC is executed between the invocation and return of `System.gc` or `WhiteBox.fullGC`, even if the call is concurrent with an active `GCLocker `. 
>> 
>> The change should also handle the issues reported in JDK-8299276.
>> 
>> Split into 3 commits, one commit for changes to each GC in [G1, Parallel, Serial]. 
>> 
>> Testing: Tier 1-5.
>
> Ivan Walulya has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup test

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?

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?

test/hotspot/jtreg/gc/TestJNICriticalStressTest.java line 234:

> 232:             now = System.currentTimeMillis();
> 233:             soFar = now - start;
> 234:         }

A single `Thread.sleep` should be fine -- the error margin should be negligible.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1153157378
PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1153159756
PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1153161634