RFR: CODETOOLS-7903021: jcstress: Dining Philosophers problem sample [v4]

[Aleksey Shipilev]

On Fri, 1 Oct 2021 23:07:07 GMT, Michael Mirwaldt <github.com+6693355+mmirwaldt at openjdk.org> wrote:

>> This s my initial draft fo a sample testing solutions for the dining philosophers problem:
>> - Wikipedia explains it on https://en.wikipedia.org/wiki/Dining_philosophers_problem
>> - It runs within 13min on my machine.
>> - It includes a short comment about the motivation for that sample and
>> a reference link to the wikipedia article about the dining philosophers problem. 
>> - It offers 3 different solutions: with monitors, with reentrant locks and with semaphores
>
> Michael Mirwaldt has updated the pull request incrementally with one additional commit since the last revision:
> 
>   improved comment and ResourceHierarchy solution in ClassicProblem_01_DiningPhilosophers

Ah yes, apologies. Please see new round of comments.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.

Should be `2021`.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 56:

> 54:     public static class ResourceHierarchy {
> 55:         private final Semaphore[] forks =
> 56:                 new Semaphore[]{new Semaphore(1), new Semaphore(1), new Semaphore(1)};

These do not need to be `Semaphores`, just plain `Object` locks, I think. `Semaphore` with a single permit is basically a mutex.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 85:

> 83:                 forks[fork1].acquire();
> 84:                 forks[fork2].acquire();
> 85:                 // eating

Shouldn't this call `tryPickForks`?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 98:

> 96: 
> 97:         protected boolean tryPickForks(int fork1, int fork2) {
> 98:             if (forks.getAndSet(fork1, 1) == 0) {

I don't believe these should be unconditional `getAndSet`, lest we want to have ABA problem here? I'd say it should be `forks.compareAndSet(fork1, 0, 1)`.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 118:

> 116:     @State
> 117:     public static class Arbitrator extends Base {
> 118:         private final Semaphore waiter = new Semaphore(1);

Same here, not a `Semaphore`, but `Object`.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 149:

> 147:                 waiter.release();
> 148: 
> 149:                 if(hasForks) {

Suggestion:

                if (hasForks) {

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_01_DiningPhilosophers.java line 196:

> 194:                 diners.release();
> 195: 
> 196:                 if(hasForks) {

Suggestion:

                if (hasForks) {

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

Changes requested by shade (Committer).

PR: https://git.openjdk.java.net/jcstress/pull/93