On Wed, 9 Dec 2020 13:28:44 GMT, Thomas Schatzl <tscha...@openjdk.org> wrote:
>> Please review this change that eliminates the use of Reference.isEnqueued by >> tests. There were three tests using it: >> >> vmTestbase/gc/gctests/ReferencesGC/ReferencesGC.java >> vmTestbase/gc/gctests/WeakReferenceGC/WeakReferenceGC.java >> jdk/java/lang/ref/ReferenceEnqueue.java >> >> In each of them, some combination of using Reference.refersTo and >> ReferenceQueue.remove with a timeout were used to eliminate the use of >> Reference.isEnqueued. >> >> I also cleaned up ReferencesGC.java in various respects. It contained >> several bits of dead code, and the failure checks were made stronger. >> >> Testing: >> mach5 tier1 >> Locally (linux-x64) ran all three tests with each GC (including Shenandoah). > > Changes requested by tschatzl (Reviewer). > [pre-existing] The topWeakReferenceGC.java description at the top describes > that the test calls System.gc() explicitly to trigger garbage collections at > the end. It does not. Maybe this could be weasel-worded around like in the > other cases in that text. There are a lot of things much more wrong with that comment. Doing more GCs doesn't cause more enqueues to happen. The "non-deterministic" enqueuing is just a race. The GC adds references to the pending list. The reference processing thread transfers references from the pending list to their associated queue (if any). The test code is racing with that. The change to use Reference.remove with a timeout eliminates all that, and one GC should be. Addressing all that would be a substantial rewrite of this test though. Mind if I defer that to a new RFE? ------------- PR: https://git.openjdk.java.net/jdk/pull/1691
