RFR: CODETOOLS-7903060: jcstress: ProducerConsumer problem sample [v3]

[Aleksey Shipilev]

On Sun, 21 Nov 2021 22:12:45 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.
>
> Michael Mirwaldt has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'openjdk:master' into ClassicProblem_02_ProducerConsumerProblem-3
>  - applied proposals from first review on sample ClassicProblem_02_ProducerConsumerProblem
>  - created first draft of sample ClassicProblem_02_ProducerConsumerProblem

Almost there.

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

> 44: 
> 45:     /*
> 46:         This sample shows you how JCStress can help you to test solutions for the famous producer-consumer problem.

The style is to have a long horizontal separator between test cases, see: https://github.com/openjdk/jcstress/blob/master/jcstress-samples/src/main/java/org/openjdk/jcstress/samples/problems/classic/Classic_01_DiningPhilosophers.java#L43

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

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

Please match the style to this: https://github.com/openjdk/jcstress/blob/master/jcstress-samples/src/main/java/org/openjdk/jcstress/samples/problems/classic/Classic_01_DiningPhilosophers.java#L45-L46

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

> 81:                 takeItemFromBuffer();
> 82:                 emptyCount.release();
> 83:                 // consume item

Leftover comment?

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

> 100: 
> 101:     @JCStressTest
> 102:     @Outcome(id = {"true"}, expect = ACCEPTABLE, desc = "One producer produced 2 items which were consumed.")

Suggestion:

    @Outcome(id = "true", expect = ACCEPTABLE, desc = "One producer produced 2 items which were consumed.")

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

> 102:     @Outcome(id = {"true"}, expect = ACCEPTABLE, desc = "One producer produced 2 items which were consumed.")
> 103:     @State
> 104:     public static class OneProducerOneConsumer extends SemaphoresBase {

Here and later: results for this test case?

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

> 136:           1, 0, 0      48.668    0,30%   Acceptable
> 137:           1, 0, 1      66.056    0,40%   Acceptable
> 138:           1, 0, 2     251.060    1,53%   Acceptable

The descriptions in this result do not match the `@Outcome` definitions below.

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

> 140:     @JCStressTest
> 141:     @Outcome(expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")
> 142:     @Outcome(id = {"0, 0, 2"}, expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")

Suggestion:

    @Outcome(id = "0, 0, 2", expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")
    @Outcome(                expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")

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

> 164:     @JCStressTest
> 165:     @Outcome(expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")
> 166:     @Outcome(id = {"0, 0, 2"}, expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")

Suggestion:

    @Outcome(id = "0, 0, 2", expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")
    @Outcome(                expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")

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

> 205:     @JCStressTest
> 206:     @Outcome(expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")
> 207:     @Outcome(id = {"0, 0, 2"}, expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")

Suggestion:

    @Outcome(id = "0, 0, 2", expect = ACCEPTABLE_INTERESTING, desc = "Producers overwrote each other's item.")
    @Outcome(                expect = ACCEPTABLE, desc = "Producers didn't overwrite each other's item.")

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

> 275:         It fails if an int overflow happens.
> 276:      */
> 277:     @SuppressWarnings("StatementWithEmptyBody")

We don't do `@SuppressWarnings` in jcstress code, I think. At least not in the samples code.

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

> 277:     @SuppressWarnings("StatementWithEmptyBody")
> 278:     @JCStressTest
> 279:     @Outcome(id = {"true"}, expect = ACCEPTABLE, desc = "One producer produced 2 items which were consumed.")

Suggestion:

    @Outcome(id = "true", expect = ACCEPTABLE, desc = "One producer produced 2 items which were consumed.")

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

> 296: 
> 297:         public void produce() {
> 298:             // produce item

Here and later: leftover comments?

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

Changes requested by shade (Committer).

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