RFR: ClassicProblem_02_ProducerConsumerProblem - third try [v2]

[Michael Mirwaldt]

On Thu, 18 Nov 2021 16:27:28 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Michael Mirwaldt has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   applied proposals from first review on sample ClassicProblem_02_ProducerConsumerProblem
>
> 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.

I have moved it there.

> 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.

I have replaced the Javadoc comments by block comments everywhere in the sample.

> 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`?

That was an accident. I have removed it.

> 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?

They are placeholders for code which was left out because it isn't needed for the test in JCStress. However, users of JCStress must not forget to add code there if they want to implement e.g. a queue with a buffer. The solutions in the sample are only the "bones" of the algorithms. The "muscles" are missing. (Excuse my strange metaphor here)

> 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.

I have added one and used it consistently in the other sub samples. I am not sure if you like it. Please feel free to tell me a better one if one comes to your mind,

> 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`.

Really? Sorry, I am confused. I remember the dining philosophers sample where you preferred p1(), p2() and p3() to philosopher1(), philosopher2() and philosopher3(). You asked me to change the names and I did it because I thought you considered those names were too long. 

Anyway, I have changed all p() by producer() and p1()/p2() by producer1()/producer2(). I have also replaced c() by consumer().

> 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.

I have added a description.

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

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