On Wed, 24 Apr 2024 07:01:52 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Christoph Langer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java >> >> Co-authored-by: Andrey Turbanov <turban...@gmail.com> > > test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 154: > >> 152: >> 153: // if Keep-Alive-SocketCleaner consumes more than >> 50% of cpu then we >> 154: // can assume a recursive loop. > > Interesting test case. I'm a bit surprised we haven't seen this > intermittently fail. Speaking of which, I see that the change to this test > proposes to remove it from `/othervm` and potentially run it in agent vm > mode. Given the kind of checks this test is doing, I think it's better to let > it run as a `othervm` test to keep the test as isolated as possible. I've thought about this, too. However, I see the only critical point why it could merit a `/othervm` test is this thing with querying the thread CPU time of the Keep-Alive-SocketCleaner thread. But I think the likelihood of this failing is the same within standard test and `/othervm`. So I'd prefer to change to standard test, since this is one of the improvements of this change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579054775