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

pop_opt, peek, peek_opt functions for queue #92

Merged
merged 2 commits into from
Nov 5, 2023

Conversation

lyrm
Copy link
Collaborator

@lyrm lyrm commented Sep 7, 2023

This PR renames and adds functions to the single consumer single producer queue, the michael-scott queue, the treiber stack and the multiple producer single consumer queue.

  • pop is renamed pop_opt
  • the new pop raises Empty if the queue is empty
  • peek and peek_opt have been added as well (except for the stack)

Other notes :

  • About work stealing deque : I did not add the peek functions, as it seems useless (it is useful mostly with a single consumer).
  • About michael-scott queue : I removed clean_until function as it was no atomic.

TODO :

  • Rename pop to pop_opt for relaxed-queue,
  • Rename pop to pop_opt for the work-stealing deque
  • Answer the question : should pop and pop_opt be almost copy-paste function or should we use the inline keyword (see comment in code)

Copy link
Collaborator Author

@lyrm lyrm left a comment

Choose a reason for hiding this comment

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

Comments about how and where to use [@inline].

Comment on lines +51 to +61
let b = Backoff.create () in
let rec loop () =
let old_head = Atomic.get head in
match Atomic.get old_head with
| Nil -> raise Empty
| Next (value, next) when Atomic.compare_and_set head old_head next -> value
| _ ->
Backoff.once b;
loop ()
in
loop ()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the same function as pop_opt except for very small changes. Should I use [@inline] and a generic pop function ?

Comment on lines 51 to 55
let[@inline] try_push t element =
try
push t element;
true
with Full -> false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here I try the [@inline]. But I don't think this is the right way to use it. Any help will be welcomed.

Copy link
Collaborator

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

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

LGTM. Minor typo below.


@raise Empty if [q] is empty. *)

val pop_opt : 'a t -> 'a option
Copy link
Collaborator

Choose a reason for hiding this comment

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

Function name is not edited in the line below.

@Sudha247 Sudha247 added this to the 1.0 milestone Nov 3, 2023
@lyrm lyrm merged commit 5b5d644 into ocaml-multicore:main Nov 5, 2023
2 checks passed
@lyrm lyrm mentioned this pull request Jan 24, 2024
12 tasks
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