On Fri, 18 Aug 2023 08:15:58 GMT, Viktor Klang <d...@openjdk.org> wrote:

>> The usual tiny improvements, with no guarantee that the intermittent test 
>> failure is actually fixed.
>
> test/jdk/java/util/concurrent/SynchronousQueue/Fairness.java line 41:
> 
>> 39:         throws Throwable
>> 40:     {
>> 41:         int threadCount = ThreadLocalRandom.current().nextInt(2, 8);
> 
> @Martin-Buchholz I'm a bit weary about making the number of threads 
> random—might make it tricky to get to reproduce if the test fails?

Suppose the test fails only with a specific number of threads.  I would want to 
know that!  At least once in the past a j.u.c. bug has been found due to 
randomization of concurrency level in a test.  I can't understand the current 
industry obsession with deterministic tests, except that too many  
organizations don't commit to hunting down flakes.

Here the error detail has been changed to report the threadcount, in case of 
failure.

> test/jdk/java/util/concurrent/SynchronousQueue/Fairness.java line 55:
> 
>> 53:             Thread t = new Thread(put);
>> 54:             t.start();
>> 55:             ts.add(t);
> 
> @Martin-Buchholz Any specific reason to add the thread *after* it has been 
> started?

I've never considered that the order would matter - so, no reason.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15337#discussion_r1299024335
PR Review Comment: https://git.openjdk.org/jdk/pull/15337#discussion_r1299027961

Reply via email to