-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add simple examples for MS queue #59
base: main
Are you sure you want to change the base?
Conversation
@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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @nikochiko! This is a great start, leaving some suggestions below -
- We can perhaps start with something simpler; just a single push and a pop.
- Then proceed to show concurrent pushes, concurrent pops, and later a combination of both.
examples/michael_scott_queue.ml
Outdated
let d1 = Domain.spawn(fun _ -> push_work_pop ms_q 1) in | ||
let d2 = Domain.spawn(fun _ -> push_work_pop ms_q 2) in | ||
let d3 = Domain.spawn(fun _ -> push_work_pop ms_q 3) in | ||
Domain.join d1; Domain.join d2; Domain.join d3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct, we can use Arrays or Lists to make this easier to manage --
let domains = Array.init 4 (fun _ -> Domain.spawn(...)) in
...
Array.iter Domain.join domains
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks much better yes.
@Sudha247 Thanks for the review. I've pushed a fix with some more examples.
|
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).
I agree, we don't need print statements for all functions.
The popper explicitly mentions that it's going to keep looping until it pops an element. So, I think this is okay. |
Nice work, thanks ! Here is a few idea that could make good use of your prints :
|
…ix-and-tweaks Fix space leaks of Michael-Scott queue and avoid the impossible
I realized my previous attempt suffered from pretty much the same race condition as the earlier implementations. The logic of the new tail update algorithm is as follows: > The tail update is only responsible for updating the tail as long as the new > node added still has the true tail. At any point some newly added node has the > true tail, so this responsibility ensures that the tail gets updated. To check > whether we are still responsible, we compare: `get new_tail == Nil`. If that > is false, then someone else is responsible and we can stop. The current > old_tail must be read before that check. If the `compare_and_set` fails, we > must retry. Otherwise we can stop as we've done our part.
Set QCHECK_MSG_INTERVAL to avoid clutter in CI logs
Mark alcotest as a test dependency
Add better print statements, accompanied by a pusher/popper id. Add a do_work function that calls Domain.cpu_relax a random large number of times. This makes concurrency apparent after the example is run.
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. |
…ue-tail-update Fix the lock-free update of Michael-Scott style queue tail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thanks for the review. |
This code of conduct lives in [ocaml/code-of-conduct](https://github.com/ocaml/code-of-conduct) and has been discussed in [this thread](https://discuss.ocaml.org/t/ocaml-community-code-of-conduct/10494). It has been adopted by the OCaml compiler and many platform tools. After a discussion we propose adopting it for lockfree.
…nduct Adopt OCaml Code of Conduct
…rking architecture.
Require qcheck-stm.0.2 and remove pin
- Change data structures names for user-ergonomy (treiber_stack -> stack for example) - Uniformize queue interface and doc - Add CONTRIBUTING file and complete test/README.md
Renaming lockfree
Refactoring to have a lockfree package
…lease-0.4 Prepare release
Remove .merlin and .ocp-indent files.
Rename `Lockfree` module in `saturn_lockfree` package to `Saturn_lockfree`.
Add a barrier module in `tests` to replace the use of `Semaphore`.
…e/saturn into ms_queue_example
I've rebased the PR to reflect updates in the repo and the name change. Will merge once the CI is happy. |
This pull request adds a simple example for the MS queue