RFR: 8287596: Reorg jdk.test.lib.util.ForceGC [v10]

Roger Riggs rriggs at openjdk.org
Thu Jun 30 15:57:45 UTC 2022


On Sat, 18 Jun 2022 05:55:32 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> This is a follow up update per comments in [JDK-8287384 PR](https://github.com/openjdk/jdk/pull/8907).  The tier1 and tier2 test in open part looks good to me.  Please help to run Mach5 just case the closed test cases are impacted.
>
> Xue-Lei Andrew Fan has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits:
> 
>  - Master
>  - use Reference.refersTo
>  - remove trailing whitespaces
>  - use timeout factor
>  - Merge
>  - Merge master
>  - Merge
>  - add timeout parameter
>  - rollback is not in this branch
>  - rollback JDK-8287384
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/47b86690...0f196282

Sorry for the delay.

test/lib/jdk/test/lib/util/ForceGC.java line 37:

> 35:     // The jtreg testing timeout factor.
> 36:     private static final double TIMEOUT_FACTOR = Double.valueOf(
> 37:             System.getProperty("test.timeout.factor", "1.0"));

If you don't mind the dependency, you can use jdk.test.lib.Utils.TIMEOUT_FACTOR
or `Utils.adjustTimeout(xx)` in the usage below.

test/lib/jdk/test/lib/util/ForceGC.java line 42:

> 40:      * Causes the current thread to wait until the {@code booleanSupplier}
> 41:      * returns true, or a specific waiting time elapses.  The waiting time
> 42:      * is 1 second scalled with the jtreg testing timeout factor.

typo: "scalled" -> "scaled"

test/lib/jdk/test/lib/util/ForceGC.java line 58:

> 56:         Reference.reachabilityFence(ref);
> 57: 
> 58:         for (int retries = (int)(timeout / 200); retries >= 0; retries--) {

The logic around the timeout might be clearer if it was only based on the number of retries,
and can be scaled by the TIMEOUT_FACTOR too.

test/lib/jdk/test/lib/util/ForceGC.java line 70:

> 68:                 // But it is fine.  For most cases, the 1st GC is sufficient
> 69:                 // to trigger and complete the cleanup.
> 70:                 queue.remove(200L);

If `remove()` returns a non-null value, then it is safe to break out of the loop.
The GC has cleared the ref. And the final `getAsBoolean()` below will return the condition.

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

PR: https://git.openjdk.org/jdk/pull/8979


More information about the core-libs-dev mailing list