On Tue, 21 Mar 2023 11:56:57 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> test/jdk/java/util/concurrent/CompletableFuture/CompletableFutureOrTimeoutExceptionallyTest.java >> line 28: >> >>> 26: * @bug 8303742 >>> 27: * @summary CompletableFuture.orTimeout can leak memory if completed >>> exceptionally >>> 28: * @run junit/othervm/timeout=1000 -Xmx128m >>> CompletableFutureOrTimeoutExceptionallyTest >> >> Hello Viktor, a timeout of 1000 seconds is 16 minutes. This timeout value is >> then multiplied by the timeout factor (which is a jtreg option) to decide >> the final timeout. The CI on which this was reported use a timeout factor of >> 4. So this would translate to a timeout of 64 minutes, which I think is >> extremely high (although it's just the upper bound). >> >> Perhaps you could increase this timeout to maybe 5 minutes (300 seconds) and >> run some tests on our CI to see if that is sufficient? > > If there isn't any value running this test with a debug build then you could > have it skip (`@requires vm.debug == false`) to save resources. > (fixed typo in original message) > @AlanBateman @pavelrappo Does the new approach seem acceptable? I don't have a strong opinion on this. The advantage is that the test is probably sub-second as it just needs one CF to complete exceptionally. The disadvantage is that it the test would need to be updated if the implementation changes, but it's not hard. So good with the approach if you want. As Jai mentioned, you can use @modules to open java.base/java.util.concurrent for deep reflectively. I would probably trim the overly long lines as they currently wrap when looking at the diffs. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13116#discussion_r1146081903
