Skip to content

added files for fine-grained linked list as a set#79

Open
DeltaCube23 wants to merge 1 commit into
ocaml-multicore:mainfrom
DeltaCube23:finelist
Open

added files for fine-grained linked list as a set#79
DeltaCube23 wants to merge 1 commit into
ocaml-multicore:mainfrom
DeltaCube23:finelist

Conversation

@DeltaCube23

Copy link
Copy Markdown

Implementation of a Fine-grained linked list (as a set) based on optimistic synchronisation referred from section 9.6 in art of multiprocessor programming. The add function requires only 1 lock, and the remove function requires 2 locks. The linked list works like a set, with elements arranged in ascending order.

Comment thread src/fine_list.ml
match temp with
| Some _ when temp == cur && prev == verify -> insert prev
| None when cur = None && prev == verify -> insert prev
| _ ->

@polytypic polytypic Jul 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could also use a simple conditional

if prev == verify && cur == verify.next then
  insert prev
else
  ...

here. This will also be slightly faster, because there is no need to distinguish between Some _ and None.

Comment thread src/fine_list.ml
let add t value =
(* x will point to the new node after insertion *)
let insert x =
if x.value = value && not (x == t.head) then (

@polytypic polytypic Jul 13, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is usually best to give the notion of equality over values as a parameter, i.e. there should be something like t.eq x.value value here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually you want t.compare x.value value = 0, because the values need to be ordered.

Comment thread src/fine_list.ml
| Some node -> node
| _ ->
is_tail := true;
create_node value None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm... Is the node created here used for something?

Comment thread src/fine_list.ml
is_tail := true;
create_node value None
in
if !is_tail then (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you avoid allocating a ref cell for the is_tail condition by inlining the continuation branches to the match:

match cur with
| None ->
    Mutex.unlock prev.lock;
    false
| Some to_find ->
    Mutex.lock to_find.lock;
    let verify = find_previous_remove t value in
    (* ... *)

Comment thread src/fine_list.ml
let temp = verify.next in
match temp with
| Some _ when temp == cur && prev == verify ->
let res = to_find.value = value in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe the logic around here could be simplified and the equality check can be moved outside of the critical section. Something like this:

      let verify = find_previous_remove t value in
      if prev == verify && cur == verify.next then begin
          Mutex.unlock prev.lock;
          Mutex.unlock to_find.lock;
          to_find.value = value (* better to use user defined equality *)
      end
      else begin
          Mutex.unlock prev.lock;
          Mutex.unlock to_find.lock;
          let again = find_previous_remove t value in
          validate again again.next
      end

The common prefix of both branches could also be factored out.

Comment thread src/fine_list.ml
let find_previous_add t key =
let rec aux prev next =
match next with
| Some node when node.value > key -> prev

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm... I wonder whether the logic and add and remove could be modified slightly so that only a single find_previous helper would be needed without impacting performance.

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