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

Reply via email to