Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out [v2]

2023-11-16 Thread Aleksey Shipilev
On Tue, 14 Nov 2023 17:25:42 GMT, Darragh Clarke  wrote:

>> Currently the `IsAlive` test occasionally times out.
>> 
>> This PR changes the timeout from `10` to `25` which I think is an adequate 
>> increase based on the failures I've seen. Though I'd be happy to change it 
>> to another value if someone thinks `25` isn't ideal.
>
> Darragh Clarke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use default timeout

Yeah, OK. IIRC, we did the `/timeout=10` initially so that the eventual bug 
case would take less than 240*TIMEOUT_FACTOR seconds to complete and fail. But 
given the non-buggy execution should actually pass, and on overloaded machines 
it could take a while to get to termination, just using the default is fine.

-

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16659#pullrequestreview-1734368948


Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out [v2]

2023-11-15 Thread David Holmes
On Tue, 14 Nov 2023 17:25:42 GMT, Darragh Clarke  wrote:

>> Currently the `IsAlive` test occasionally times out.
>> 
>> This PR changes the timeout from `10` to `25` which I think is an adequate 
>> increase based on the failures I've seen. Though I'd be happy to change it 
>> to another value if someone thinks `25` isn't ideal.
>
> Darragh Clarke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use default timeout

Seems fine. If it ever times out again then we know we have a real problem. :)

I suspect, as  Alan suggested, the `othervm` could be dropped, but I'm not 
concerned about it staying.

Thanks

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16659#pullrequestreview-1733586933


Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out [v2]

2023-11-14 Thread Alan Bateman
On Tue, 14 Nov 2023 17:25:42 GMT, Darragh Clarke  wrote:

>> Currently the `IsAlive` test occasionally times out.
>> 
>> This PR changes the timeout from `10` to `25` which I think is an adequate 
>> increase based on the failures I've seen. Though I'd be happy to change it 
>> to another value if someone thinks `25` isn't ideal.
>
> Darragh Clarke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use default timeout

Marked as reviewed by alanb (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16659#pullrequestreview-1730325897


Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out [v2]

2023-11-14 Thread Darragh Clarke
> Currently the `IsAlive` test occasionally times out.
> 
> This PR changes the timeout from `10` to `25` which I think is an adequate 
> increase based on the failures I've seen. Though I'd be happy to change it to 
> another value if someone thinks `25` isn't ideal.

Darragh Clarke has updated the pull request incrementally with one additional 
commit since the last revision:

  use default timeout

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16659/files
  - new: https://git.openjdk.org/jdk/pull/16659/files/cd13a00c..d3ac2694

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=16659=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=16659=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16659.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16659/head:pull/16659

PR: https://git.openjdk.org/jdk/pull/16659


Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out

2023-11-14 Thread Daniel Fuchs
On Tue, 14 Nov 2023 16:41:31 GMT, Darragh Clarke  wrote:

> Currently the `IsAlive` test occasionally times out.
> 
> This PR changes the timeout from `10` to `25` which I think is an adequate 
> increase based on the failures I've seen. Though I'd be happy to change it to 
> another value if someone thinks `25` isn't ideal.

Looks reasonable to me. Let's give it at least 48 hours before integrating 
though, to let folks who might have an opinion (@dholmes-ora ?) to chime in.

-

Marked as reviewed by dfuchs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16659#pullrequestreview-1730279358


Re: RFR: 8317834: java/lang/Thread/IsAlive.java timed out

2023-11-14 Thread Alan Bateman
On Tue, 14 Nov 2023 16:41:31 GMT, Darragh Clarke  wrote:

> Currently the `IsAlive` test occasionally times out.
> 
> This PR changes the timeout from `10` to `25` which I think is an adequate 
> increase based on the failures I've seen. Though I'd be happy to change it to 
> another value if someone thinks `25` isn't ideal.

test/jdk/java/lang/Thread/IsAlive.java line 28:

> 26:  * @bug 8305425
> 27:  * @summary Check Thread.isAlive
> 28:  * @run main/othervm/timeout=25 IsAlive

Can it just run with the default timeout, meaning drop /timeout=N ? Also does 
this test need really need to run in othervm mode?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16659#discussion_r1392935779


RFR: 8317834: java/lang/Thread/IsAlive.java timed out

2023-11-14 Thread Darragh Clarke
Currently the `IsAlive` test occasionally times out.

This PR changes the timeout from `10` to `25` which I think is an adequate 
increase based on the failures I've seen. Though I'd be happy to change it to 
another value if someone thinks `25` isn't ideal.

-

Commit messages:
 - bumped timeout from 10 to 25

Changes: https://git.openjdk.org/jdk/pull/16659/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=16659=00
  Issue: https://bugs.openjdk.org/browse/JDK-8317834
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16659.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16659/head:pull/16659

PR: https://git.openjdk.org/jdk/pull/16659