On Tue, 21 Mar 2023 11:02:00 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Improves the stability of the memory leak test for CompletableFuture timeout >> cancellation by both reducing the count by 50% (which should still be above >> threshold to trigger given the ample margin set initially) as well as >> extending the default timeout of the test run. > > 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? @jaikiran Having a long timeout doesn't seem like a problem given that it just needs enough time to run through the iterations (i.e. that's the max duration of the test before giving up). @AlanBateman That's a great observation—do you see anything about this which would impact whether it makes sense to run on top of a debug-build (I don't see any). If it doesn't need to run on a debug build then I'll add the requirement you suggested. Let me know. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13116#discussion_r1143522447
