RFR: 8057586: Explicit GC ignored if GCLocker is active

[Ivan Walulya]

On Tue, 28 Mar 2023 07:30:57 GMT, Albert Mingkun Yang <ayang 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.
>
> test/hotspot/jtreg/gc/TestJNICriticalStressTest.java line 132:
> 
>> 130: 
>> 131:             byte[] outputArray = new byte[2 * inputArray.length];
>> 132:             deflater.deflate(outputArray);
> 
> "To release resources used by this Deflater, the [end()](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/zip/Deflater.html#end()) method should be called explicitly."
> 
> Why is it safe to not call it here?

will add the `end()` call. Thanks.

> test/hotspot/jtreg/gc/TestJNICriticalStressTest.java line 208:
> 
>> 206:         println("Starting " + allocThreadNum + " allocating threads");
>> 207:         for (int i = 0; i < allocThreadNum; i += 1) {
>> 208:             new Thread(new AllocatingWorker()).start();
> 
> What's the point of having an alloc thread? I'd expect whiteboxapi to be enough to trigger gc cycles regardless of heap state.

https://bugs.openjdk.org/browse/JDK-8057573

Reproducer taken from here. Added minor changes to fit JTREG.

The alloc thread test makes for more robust testing, but not required.

> test/hotspot/jtreg/gc/TestJNICriticalStressTest.java line 234:
> 
>> 232: 
>> 233:         long gcCountAfter = collector.getCollectionCount();
>> 234:         Asserts.assertLessThanOrEqual(gcCountBefore + fullGcCounts, gcCountAfter, "Triggered more Full GCs than expected");
> 
> Could this assert be placed right after `wb.fullGC();` so that the failure is closer to its cause?

yes it can.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1150244703
PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1150243866
PR Review Comment: https://git.openjdk.org/jdk/pull/13191#discussion_r1150244219