CODETOOLS-7903060: jcstress: ProducerConsumer problem sample #104

mmirwaldt · 2021-11-14T22:56:25Z

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.

Progress

Change must not contain extraneous whitespace
Change must be properly reviewed

Issue

CODETOOLS-7903060: jcstress: ProducerConsumer problem sample

Reviewers

Aleksey Shipilev (@shipilev - Committer) ⚠️ Review applies to 14a1753

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jcstress pull/104/head:pull/104
$ git checkout pull/104

Update a local copy of the PR:
$ git checkout pull/104
$ git pull https://git.openjdk.java.net/jcstress pull/104/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 104

View PR using the GUI difftool:
$ git pr show -t 104

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jcstress/pull/104.diff

bridgekeeper · 2021-11-14T22:56:32Z

👋 Welcome back mmirwaldt! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2021-11-14T23:01:08Z

Webrevs

mmirwaldt · 2021-11-15T08:59:54Z

This PR looks much better now. Thanks for the advice, @shipilev .
I will do my next PRs that way, too.

Can you review this sample, please, @shipilev?

shipilev

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.

.../openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java

…cerConsumerProblem

mmirwaldt · 2021-11-18T22:55:30Z

I must confess I am not sure how much description is enough for you and what is too much. It appeared to me in the last PRs you sometimes prefer rather a minimum than a maximum of explainations to avoid useless comments about obvious things. Maybe I am wrong.
What do you think about telling me where you want more description?
I don't want to waste your time (and mine neither) with too long texts.
Or do you really prefer the opposite? (I mean you tell me what I must remove from the texts)

Btw: I always try to be short on my answers for you because I believe you don't like to read long texts from me.

mmirwaldt · 2021-11-18T23:01:13Z

Maybe it's a good idea to review again the sample ClassicProblem_01_DiningPhilosophers after this PR.
It has still got some "flaws" which you mentioned in this sample. We should be consistent in all samples.

shipilev · 2021-11-19T11:48:00Z

Please change the PR title to "CODETOOLS-7903060: jcstress: ProducerConsumer problem sample" to get it hooked properly.

shipilev

I have just merged #105 that brings consistency to all current samples. Please pull from master, move your sample in the new location and try to match the style :)

.../openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java

mmirwaldt · 2021-11-21T22:07:33Z

Please change the PR title to "CODETOOLS-7903060: jcstress: ProducerConsumer problem sample" to get it hooked properly.

Done.

…Problem-3

openjdk · 2021-11-21T22:08:17Z

@mmirwaldt this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout ClassicProblem_02_ProducerConsumerProblem-3
git fetch https://git.openjdk.java.net/jcstress master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

shipilev

Almost there.

.../openjdk/jcstress/samples/concurrency/classic/ClassicProblem_02_ProducerConsumerProblem.java

…Problem-3

…rConsumerProblem

…ConsumerProblem

shipilev

Still not there. Please match the style of the previous samples exactly.

...n/java/org/openjdk/jcstress/samples/problems/classic/Classic_02_ProducerConsumerProblem.java

mmirwaldt · 2021-12-09T20:31:01Z

Which results?
I cannot see any

          RESULT        SAMPLES     FREQ      EXPECT  DESCRIPTION
            true  6,325,295,104  100.00%  Acceptable  Trivial.

for other tests than FlawedTwoProducersOneConsumer.
I can only see

(ETA: in 00:01:40; at Do., 2021-12-09 21:14:35)
(Sampling Rate: 8,09 M/sec)
(JVMs: 0 starting, 4 running, 1 finishing)
(CPUs: 12 configured, 12 allocated)
(Results: 434 planned; 359 passed, 0 failed, 0 soft errs, 0 hard errs)

How can I enable this output for all other tests?

I have added the separators but I miss the results.

…rConsumerProblem

mmirwaldt · 2021-12-09T21:11:42Z

I am confused. Some integration tests don't run anymore. I cannot imagine any last change causing that.

shipilev · 2021-12-10T10:53:46Z

How can I enable this output for all other tests?
I have added the separators but I miss the results.

Say -v to jcstress invocation: java -jar jcstress.jar ... -v.

…ConsumerProblem

mmirwaldt · 2021-12-10T14:08:33Z

How can I enable this output for all other tests?
I have added the separators but I miss the results.

Say -v to jcstress invocation: java -jar jcstress.jar ... -v.

Ah thank you, @shipilev!

I have added the results for all the other tests.

Please feel free to ask for further changes.
If I have accidentally resolved one of your changes although it's still wrong,
then please be patient with me and unresolve that issue with a comment.

mmirwaldt · 2021-12-28T14:24:24Z

Any update, @shipilev?

vlsi · 2021-12-29T06:37:59Z

...n/java/org/openjdk/jcstress/samples/problems/classic/Classic_02_ProducerConsumerProblem.java

+        void producer() {
+            produce();
+            produce();
+        }
+
+        @Actor
+        void consumer() {
+            consume();
+            consume();
+        }
+
+        @Arbiter
+        public void fake(Z_Result r) {
+            r.r1 = true;


@mmirwaldt , It does not seem to be good to have "all outcomes are acceptable".

Have you considered producing messages with different values so you can tell which one was consumed?

For instance: r1=produce(1); r2=produce(2); and then r3=consume(); r4=consume();.

Then you could verify:

"1, 2, 1, 2" is acceptable

all others states are forbidden.

The same would apply to the other samples.

WDYT?

shipilev

All right, this is acceptable.

openjdk · 2022-01-03T10:49:46Z

⚠️ @mmirwaldt the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout ClassicProblem_02_ProducerConsumerProblem-3
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

openjdk · 2022-01-03T10:49:48Z

@mmirwaldt This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

7903060: jcstress: ProducerConsumer problem sample

Reviewed-by: shade

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@shipilev) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mmirwaldt · 2022-01-11T00:01:27Z

/integrate

openjdk · 2022-01-11T00:02:22Z

@mmirwaldt
Your change (at version f121d15) is now ready to be sponsored by a Committer.

shipilev · 2022-01-11T06:41:13Z

/sponsor

openjdk · 2022-01-11T06:42:17Z

Going to push as commit 0f28604.

openjdk · 2022-01-11T06:42:33Z

@shipilev @mmirwaldt Pushed as commit 0f28604.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

created first draft of sample ClassicProblem_02_ProducerConsumerProblem

f611c81

openjdk bot added the rfr Pull request is ready for review label Nov 14, 2021

mmirwaldt marked this pull request as draft November 14, 2021 23:39

mmirwaldt marked this pull request as ready for review November 14, 2021 23:39

shipilev requested changes Nov 18, 2021

View reviewed changes

applied proposals from first review on sample ClassicProblem_02_Produ…

25fd3bb

…cerConsumerProblem

mmirwaldt requested a review from shipilev November 18, 2021 22:59

shipilev requested changes Nov 19, 2021

View reviewed changes

mmirwaldt changed the title ~~ClassicProblem_02_ProducerConsumerProblem - third try~~ CODETOOLS-7903060: jcstress: ProducerConsumer problem sample Nov 21, 2021

Merge branch 'openjdk:master' into ClassicProblem_02_ProducerConsumer…

8ddd726

…Problem-3

openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed merge-conflict Pull request has merge conflict with target branch labels Nov 21, 2021

shipilev requested changes Nov 30, 2021

View reviewed changes

mmirwaldt added 3 commits December 9, 2021 01:29

Merge branch 'openjdk:master' into ClassicProblem_02_ProducerConsumer…

35963e2

…Problem-3

applied proposals from the second review on sample Classic_02_Produce…

d0956b9

…rConsumerProblem

applied proposals from the third review on sample Classic_02_Producer…

61434a8

…ConsumerProblem

mmirwaldt requested a review from shipilev December 9, 2021 01:47

shipilev requested changes Dec 9, 2021

View reviewed changes

applied proposals from the fourth review on sample Classic_02_Produce…

e34a182

…rConsumerProblem

applied proposals from the fifth review on sample Classic_02_Producer…

14a1753

…ConsumerProblem

mmirwaldt requested a review from shipilev December 23, 2021 13:13

vlsi reviewed Dec 29, 2021

View reviewed changes

shipilev approved these changes Jan 3, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 3, 2022

Setting my user name and email

f121d15

openjdk bot added the sponsor Pull request is ready to be sponsored label Jan 11, 2022

openjdk bot added the integrated Pull request has been integrated label Jan 11, 2022

openjdk bot closed this Jan 11, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jan 11, 2022

mmirwaldt deleted the ClassicProblem_02_ProducerConsumerProblem-3 branch January 11, 2022 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CODETOOLS-7903060: jcstress: ProducerConsumer problem sample #104

CODETOOLS-7903060: jcstress: ProducerConsumer problem sample #104

mmirwaldt commented Nov 14, 2021 •

edited by openjdk bot

bridgekeeper bot commented Nov 14, 2021

mlbridge bot commented Nov 14, 2021 •

edited

mmirwaldt commented Nov 15, 2021 •

edited

shipilev left a comment

mmirwaldt commented Nov 18, 2021 •

edited

mmirwaldt commented Nov 18, 2021

shipilev commented Nov 19, 2021

shipilev left a comment

mmirwaldt commented Nov 21, 2021

openjdk bot commented Nov 21, 2021

shipilev left a comment

shipilev left a comment

mmirwaldt commented Dec 9, 2021

mmirwaldt commented Dec 9, 2021

shipilev commented Dec 10, 2021

mmirwaldt commented Dec 10, 2021 •

edited

mmirwaldt commented Dec 28, 2021

vlsi Dec 29, 2021

shipilev left a comment

openjdk bot commented Jan 3, 2022

openjdk bot commented Jan 3, 2022 •

edited

mmirwaldt commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

shipilev commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

CODETOOLS-7903060: jcstress: ProducerConsumer problem sample #104

CODETOOLS-7903060: jcstress: ProducerConsumer problem sample #104

Conversation

mmirwaldt commented Nov 14, 2021 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Nov 14, 2021

mlbridge bot commented Nov 14, 2021 • edited

Webrevs

mmirwaldt commented Nov 15, 2021 • edited

shipilev left a comment

Choose a reason for hiding this comment

mmirwaldt commented Nov 18, 2021 • edited

mmirwaldt commented Nov 18, 2021

shipilev commented Nov 19, 2021

shipilev left a comment

Choose a reason for hiding this comment

mmirwaldt commented Nov 21, 2021

openjdk bot commented Nov 21, 2021

shipilev left a comment

Choose a reason for hiding this comment

shipilev left a comment

Choose a reason for hiding this comment

mmirwaldt commented Dec 9, 2021

mmirwaldt commented Dec 9, 2021

shipilev commented Dec 10, 2021

mmirwaldt commented Dec 10, 2021 • edited

mmirwaldt commented Dec 28, 2021

vlsi Dec 29, 2021

Choose a reason for hiding this comment

shipilev left a comment

Choose a reason for hiding this comment

openjdk bot commented Jan 3, 2022

openjdk bot commented Jan 3, 2022 • edited

mmirwaldt commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

shipilev commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

openjdk bot commented Jan 11, 2022

mmirwaldt commented Nov 14, 2021 •

edited by openjdk bot

mlbridge bot commented Nov 14, 2021 •

edited

mmirwaldt commented Nov 15, 2021 •

edited

mmirwaldt commented Nov 18, 2021 •

edited

mmirwaldt commented Dec 10, 2021 •

edited

openjdk bot commented Jan 3, 2022 •

edited