Skip to content
Snippets Groups Projects
Commit fff7241a authored by Nadav Har'El's avatar Nadav Har'El
Browse files

Removed std::atomics from read size of lock-free queue

As Avi suggested in his review, let's not have the read-size memory
ordering code in the queue implementation - just in the caller if it
is needed (and usually, it won't be needed).
parent d956b1a6
No related branches found
No related tags found
No related merge requests found
...@@ -8,7 +8,8 @@ ...@@ -8,7 +8,8 @@
// //
// Note that while there's just one concurrent consumer - it does not have // Note that while there's just one concurrent consumer - it does not have
// to always be the same thread or processor - it's just that the program // to always be the same thread or processor - it's just that the program
// logic guarantees that in no case do two pop()s get called concurrently. // logic guarantees that in no case do two pop()s get called concurrently,
// and the caller is responsible to ensure the appropriate memory ordering.
// //
// This is one of the simplest, and most well-known (and often // This is one of the simplest, and most well-known (and often
// reinvented) lock-free algorithm, and we actually have another // reinvented) lock-free algorithm, and we actually have another
...@@ -38,11 +39,7 @@ public: ...@@ -38,11 +39,7 @@ public:
typedef linked_item<T> LT; typedef linked_item<T> LT;
private: private:
std::atomic<LT*> pushlist; std::atomic<LT*> pushlist;
// While poplist isn't used concurrently, we still want to allow it to LT* poplist;
// be accessed from different threads, and when we read the pointer
// from poplist in one CPU we need the list it points to to also be
// visible (so-called "release/consume" memory ordering).
std::atomic<LT*> poplist;
public: public:
queue_mpsc<T>() : pushlist(nullptr), poplist(nullptr) { } queue_mpsc<T>() : pushlist(nullptr), poplist(nullptr) { }
...@@ -64,14 +61,13 @@ public: ...@@ -64,14 +61,13 @@ public:
inline bool pop(T *result) inline bool pop(T *result)
{ {
LT *head = poplist.load(std::memory_order_consume); if (poplist) {
if (head) {
// The poplist (prepared by an earlier pop) is not empty, so pop // The poplist (prepared by an earlier pop) is not empty, so pop
// from it. We don't need any locking to access the poplist, as // from it. We don't need any locking to access the poplist, as
// it is only touched by pop operations, and our assumption (of a // it is only touched by pop operations, and our assumption (of a
// single-consumer queue) is that pops cannot be concurrent. // single-consumer queue) is that pops cannot be concurrent.
*result = head->value; *result = poplist->value;
poplist.store(head->next, std::memory_order_release); poplist = poplist->next;
return true; return true;
} else { } else {
// The poplist is empty. Atomically take the entire pushlist (pushers // The poplist is empty. Atomically take the entire pushlist (pushers
...@@ -88,20 +84,18 @@ public: ...@@ -88,20 +84,18 @@ public:
// the last item (the oldest pushed item) as the result of the pop. // the last item (the oldest pushed item) as the result of the pop.
while (r->next) { while (r->next) {
LT *next = r->next; LT *next = r->next;
r->next = head; r->next = poplist;
head = r; poplist = r;
r = next; r = next;
} }
*result = r->value; *result = r->value;
poplist.store(head, std::memory_order_release);
return true; return true;
} }
} }
inline bool empty(void) const inline bool empty(void) const
{ {
return (!poplist.load(std::memory_order_relaxed) && return (!poplist && !pushlist.load(std::memory_order_relaxed));
!pushlist.load(std::memory_order_relaxed));
} }
}; };
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment