On Wed, 26 Mar 2025 14:49:23 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Brent Christian has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 'return' not needed in lambda > > test/jdk/java/lang/ref/FinalizerHistogramTest.java line 75: > >> 73: ref2.refersTo(null) && >> 74: trappedCount.intValue() > 0 >> 75: ); > > Hello Brent, I intially thought this still had the chance of intermittently > timing out waiting for this condition to be satisfied, because I think > there's no guarantee that the `MyObject` instances referred to by `ref1` and > `ref2` would be the "prioritized" ones (compared to the others that we create > in a loop) that would be eligible for GC. But then I noticed that we don't > make use of the return value from the `ForceGC.wait()` call and I think > that's a good thing. > > Having said that, I wonder if this change would now lead to more frequent > intermittent failures because unlike previously where we used to virtually > wait "forever" in the `while(wasTrapped < 1);`, with this change we now wait > for a maximum of "1 second * TIMEOUT_FACTOR". I suspect we need to add some > additional logic here which force waits for at least one > `MyObject.finalize()` to be invoked (i.e. `trappedCount` is at least 1) > before proceeding to do anything else. `ForceGC` is already waiting for `trappedCount > 0`. And I would point out that `ForceGC` will be making several `System.gc()` calls, compared to just one in the original version of the test. But if there's concern that we won't reach the needed state during `ForceGC`'s default timeout, I can call `ForceGC.waitFor()` with a longer timeout. `60s` ought to be plenty. What do you think? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24143#discussion_r2014908306
