Skip to content
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 new functions and improve documentations of Treiber stack. #158

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

lyrm
Copy link
Collaborator

@lyrm lyrm commented Oct 31, 2024

Add functions to the Treiber stack, to match bounded stack in #156 :

  • With list : of_list, push_all, pop_all
  • With seq: of_seq, add_seq, to_seq
  • Consumer functions : peek_exn, peek_opt, pop_exn, drop_exn
  • Set functions : set_exn, try_exn
  • Others : try_compare_and_pop, try_compare_and_set

@lyrm lyrm force-pushed the treiber_stack_upgrade branch from 85cdb50 to 8f72d22 Compare November 5, 2024 10:01
Copy link
Contributor

@art-w art-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very nice! I had some minor questions, but otherwise I'm curious if you have examples use-cases for try_set, try_compare_and_set and set_exn? :)

src/treiber_stack.ml Outdated Show resolved Hide resolved
src/treiber_stack.ml Outdated Show resolved Hide resolved
src/treiber_stack.mli Outdated Show resolved Hide resolved
@lyrm
Copy link
Collaborator Author

lyrm commented Nov 12, 2024

For the new functions, try_set, try_compare_and_set, and set_exn, I was considering whether they could be useful for some form of message passing.

Originally, these functions were introduced for the hashtable from the picos library (see picos PR 317). In the context of the hashtable, these operations make a lot of sense. However, due to time constraints, I haven’t had the chance to fully explore how these could be beneficial for a stack. I might move this part of the PR to a subsequent draft PR instead. What do you think ?

@art-w
Copy link
Contributor

art-w commented Nov 19, 2024

Yes I've plenty of use cases for those functions with the hashtable, but I can't think of any for the stack... so it makes sense to remove them for the time being and reconsider adding them later once a use case shows up :)

@lyrm
Copy link
Collaborator Author

lyrm commented Nov 21, 2024

@art-w : Thanks a lot for your reviews ! Anything else before I merge this ?

Copy link
Contributor

@art-w art-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@lyrm lyrm merged commit 8ed5de4 into ocaml-multicore:main Nov 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants