RFR: ClassicProblem_02_ProducerConsumerProblem - third try

[Aleksey Shipilev]

On Sun, 14 Nov 2021 22:56:25 GMT, Michael Mirwaldt <duke at openjdk.java.net> wrote:

> This sample implements some solutions from the wikipedia article about the producer-consumer-problem on https://en.wikipedia.org/wiki/Producer%E2%80%93consumer_problem .
> It contains these solutions:
> - _OneProducerOneConsumer_ with semaphores
> - _FlawedTwoProducersOneConsumer_ with semaphores in which the producers sometimes use the same index for their items and overwrite each others item in the buffer
> - _FixedTwoProducersOneConsumer_ with semaphores which uses an extra lock for the index so that the producers never use the same index for their items
> - _Lock_ doesn't use any sempahores but a reentrant lock with two conditions. It supports many producers and many consumers
> - _AtomicIntegers_ neither uses any semaphores but two atomic integers and supports one producer and one consumer.
> 
> I have decided not to implement the last solution with finite-range single writer registers in the wikipedia article. It looked far too complicated to me.
> 
> This is my third try to avoid squash commits.

I have a general comment about the style. The is sample code, it has to be fluent. Meaning, the reader has to be able to read the entire file *as prose*, being told what is shown, what are the expected results and why, what are the actual results. Look at how `Basic*` samples are written.

This, unfortunately, does not pass the bar for a good sample yet.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 42:

> 40:     How to run this test:
> 41:         $ java -jar jcstress-samples/target/jcstress.jar -t ClassicProblem_02_ProducerConsumerProblem
> 42:  */

This section is usually within the class declaration.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 46:

> 44: /**
> 45:  * This sample shows you how JCStress can help you to test solutions for the famous producer-consumer problem.
> 46:  * See https://en.wikipedia.org/wiki/Producer%E2%80%93consumer_problem for more information about the problem and solutions.

I think samples use `/* */` block comments consistently. `/** */` is Javadoc-style comment, which would barf on some toolchains that would then parse these as Javadocs.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 48:

> 46:  * See https://en.wikipedia.org/wiki/Producer%E2%80%93consumer_problem for more information about the problem and solutions.
> 47:  */
> 48: public abstract class ClassicProblem_02_ProducerConsumerProblem {

Why `abstract`?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 49:

> 47:  */
> 48: public abstract class ClassicProblem_02_ProducerConsumerProblem {
> 49:     private final static int BUFFER_SIZE = 2;

This should be inside `SemaphoresBase`?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 65:

> 63:         public int produce() {
> 64:             try {
> 65:                 // produce item

What these comments are for? How do they relate to test?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 100:

> 98: 
> 99:     @JCStressTest
> 100:     @Outcome(id = {"true"}, expect = ACCEPTABLE)

`desc`, please.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 104:

> 102:     public static class OneProducerOneConsumer extends SemaphoresBase {
> 103:         @Actor
> 104:         void p() {

Full names, please, e.g. `producer`.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 137:

> 135:      */
> 136:     @JCStressTest
> 137:     @Outcome(expect = ACCEPTABLE)

`desc`, please.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java line 250:

> 248:             lock.lock();
> 249:             try {
> 250:                 while(count == 0) {

The style is to leave space after `while`, `if`, `for`, etc.

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

Changes requested by shade (Committer).

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