Skip to content

Conversation

@dgarvit
Copy link
Collaborator

@dgarvit dgarvit commented Jun 7, 2019

No description provided.

* Created by Garvit Dewan -
* https://github.com/dgarvit/epoch-based-manager/blob/master/src/LockFreeQueue.chpl
*
* Lock-Free Queue that uses ABA feature of Distributed Data Structures
Copy link
Owner

Choose a reason for hiding this comment

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

Should probably say that it uses the ABA feature of LocalAtomicObject. Sure the LocalAtomicObject was created back in GSoC 2017, but you don't need to state that here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed it.


proc init(type eltType) {
this.eltType = eltType;
val = nil;
Copy link
Owner

Choose a reason for hiding this comment

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

Do not initialize val with a value, leave it as is (it will be initialized to a default value automatically). If eltType is an integral (int,uint, etc.), a tuple or a record, this will result in a compiler error.

Copy link
Collaborator Author

@dgarvit dgarvit Jun 8, 2019

Choose a reason for hiding this comment

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

Well, I think that would further require changes to peek() and dequeue() in terms that if queue is empty then cannot return nil for eltType int

var curr_head = _head.readABA();
var curr_tail = _tail.readABA();
var next = curr_head.next.readABA();
if (_head.read() == _tail.read()) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you read curr_head and curr_tail and then try to read them again? Are you sure you shouldn't do curr_head.getObject() == curr_tail.getObject()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad, fixed it. Thanks for pointing out!

}

iter these() : objType {
var ptr = _head.read().next.read();
Copy link
Owner

Choose a reason for hiding this comment

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

What happens if the queue is empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When queue is empty, it will hold a dummy node, which will have a next, which will be a LocalAtomicObject, which will point to a nil value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, nothing will happen, because, if ptr is nil. then the function will finish executing.

Copy link
Owner

Choose a reason for hiding this comment

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

Right, I forgot about the dummy node! That should work then.

}

proc peek() : objType {
return _head.read().next.read().val;
Copy link
Owner

Choose a reason for hiding this comment

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

What happens if the queue is empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar to the previous reason, this will return nil. There was a bug with it, which I have fixed now.

var ptr = _head.read();
while (ptr != nil) {
_head = ptr.next;
delete ptr.val;
Copy link
Owner

Choose a reason for hiding this comment

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

Nope, don't assume the user will have classes. If they want to have their classes managed, they should use a shared or owned type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh. Alright.

}
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Add a main function here that tests it, similar to what is done in LockFreeStack.chpl:

proc main() {
var a = new LockFreeStack(int);
forall i in 1..1024 do a.push(i);
a.push(1025..2048);
writeln(+ reduce a);
}

@LouisJenkinsCS
Copy link
Owner

Can you also make .compopts and .good file like I do here...

Then I can possibly run start_test on it for sanity check.

@LouisJenkinsCS
Copy link
Owner

I think that you should make dequeue and peek return a (bool, eltType). Don't return nil

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.

3 participants