saturn icon indicating copy to clipboard operation
saturn copied to clipboard

Add simple examples for MS queue

Open nikochiko opened this issue 1 year ago • 7 comments

This pull request adds a simple example for the MS queue

nikochiko avatar Mar 01 '23 09:03 nikochiko

@Sudha247 I have added a simple example with 3 domains, each performing one task that does push-work-pop, and prints what was pushed and popped.

Is this a good example and the expected usage?

Do you have some other cases in mind that might make for good examples?

nikochiko avatar Mar 01 '23 09:03 nikochiko

@Sudha247 Thanks for the review.

I've pushed a fix with some more examples.

  • Each example is organised as a self-contained function from the creation of queue till end of all operations
  • A printf is called every time an item is pushed/popped. I added it so that it will give some assurance when run that things are happening correctly, but I'm not sure about it because it feels distracting while reading the code
  • In the last example with concurrent push and pop both, n_domains/2 domains are created for both pushers and poppers, the popper is a recursive function that pops exactly one item.
  • In the last example, the popper domains are spawned before pusher domains so that it is apparent from the logs that poppers and pushers are both working at the same time. But I am not sure about this because it might make it a little confusing for the reader.

nikochiko avatar Mar 01 '23 22:03 nikochiko

This is covering more functionality now. I like how the examples are self contained - it will be easy to move this to another format of documentation/tutorial if needed (not needed now).

A printf is called every time an item is pushed/popped. I added it so that it will give some assurance when run that things are happening correctly, but I'm not sure about it because it feels distracting while reading the code

I agree, we don't need print statements for all functions.

In the last example, the popper domains are spawned before pusher domains so that it is apparent from the logs that poppers and pushers are both working at the same time. But I am not sure about this because it might make it a little confusing for the reader.

The popper explicitly mentions that it's going to keep looping until it pops an element. So, I think this is okay.

Sudha247 avatar Mar 03 '23 10:03 Sudha247

Nice work, thanks !

Here is a few idea that could make good use of your prints :

  • make domains push or pop different values or write their own id so it is possible to see in which order they have worked. Domain.self is not great for that, but you can assign an integer to each domain when spawning them.

  • adding a do_work function that gives a chance to domains to work in a non sequential order (you can use Domain.cpu_relax in a loop for example) .

lyrm avatar Mar 03 '23 23:03 lyrm

Thanks for the feedback.

I have added the changes we discussed. Removed some print statements, the ones that are there will log with a pusher/popper ID. do_work runs Domain.cpu_relax a random large number of times, so that the concurrency is apparent when the examples are run.

nikochiko avatar Apr 08 '23 07:04 nikochiko

Thanks for the review. Done and I also ran it through dune fmt because that check was failing.

nikochiko avatar May 10 '23 22:05 nikochiko

I've rebased the PR to reflect updates in the repo and the name change. Will merge once the CI is happy.

Sudha247 avatar Sep 13 '23 07:09 Sudha247