patterncppMinor
Lockless SCSP queue
Viewed 0 times
locklessqueuescsp
Problem
I'm looking for a code review on a class that I've written. It was my first attempt at lockless programming and I'd love any feedback I can get.
```
#pragma once
#include
#include
#include
namespace Pro {
namespace Util {
/*! Thread Safe Lock Free Pipe
*/
template
class alignas(64) Pipe {
protected:
size_t push_position_;
char pad_p[64 - (sizeof(size_t))]; // Producer C-Line
size_t pop_position_;
char pad_c[64 - (sizeof(size_t))]; // Consumer C-Line
T* queue_;
size_t queue_offset_;
size_t capacity_;
char pad_sr[64 - sizeof(T) - (sizeof(size_t) 2)]; // Shared Read C-Line
std::atomic size_;
char pad_sw[64 - sizeof(std::atomic)]; // Shared Write C-Line
inline bool DecrementSize(){
if(size_ (operator new(sizeof(T) * size + 64));
queue_offset_ = 64 - ((size_t)queue_ % 64);
queue_ = (T)((char)queue_ + queue_offset_);
capacity_ = size;
pop_position_ = push_position_ = size_ = 0;
}
}
//! A destroyed Pipe may be reused after calling Initialize() again.
//! There is no check inplace to determine if the Pipe is initialised for pop and pushes
inline void Destroy() {
if (queue_ == nullptr)
return;
while (Pop());
queue_ = (T)((char)queue_ - queue_offset_);
operator delete(queue_);
queue_ = nullptr;
}
// Issue if pushing while poping with only one element
inline bool Push(const T& obj) {
if(capacity_ - size_ == 0)
return false;
auto pos = push_position_++ % capacity_;
new(reinterpret_cast(queue_) + pos) T(obj);
```
#pragma once
#include
#include
#include
namespace Pro {
namespace Util {
/*! Thread Safe Lock Free Pipe
*/
template
class alignas(64) Pipe {
protected:
size_t push_position_;
char pad_p[64 - (sizeof(size_t))]; // Producer C-Line
size_t pop_position_;
char pad_c[64 - (sizeof(size_t))]; // Consumer C-Line
T* queue_;
size_t queue_offset_;
size_t capacity_;
char pad_sr[64 - sizeof(T) - (sizeof(size_t) 2)]; // Shared Read C-Line
std::atomic size_;
char pad_sw[64 - sizeof(std::atomic)]; // Shared Write C-Line
inline bool DecrementSize(){
if(size_ (operator new(sizeof(T) * size + 64));
queue_offset_ = 64 - ((size_t)queue_ % 64);
queue_ = (T)((char)queue_ + queue_offset_);
capacity_ = size;
pop_position_ = push_position_ = size_ = 0;
}
}
//! A destroyed Pipe may be reused after calling Initialize() again.
//! There is no check inplace to determine if the Pipe is initialised for pop and pushes
inline void Destroy() {
if (queue_ == nullptr)
return;
while (Pop());
queue_ = (T)((char)queue_ - queue_offset_);
operator delete(queue_);
queue_ = nullptr;
}
// Issue if pushing while poping with only one element
inline bool Push(const T& obj) {
if(capacity_ - size_ == 0)
return false;
auto pos = push_position_++ % capacity_;
new(reinterpret_cast(queue_) + pos) T(obj);
Solution
Move Operators:
Much easier to write in terms of swap. Then you only need to write it once (in the swap method).
Also Best to mark move constructor and move assignment as
You can then write your own swap very easily.
Variable declaration
Variable declaration order is important.
Objects will always be constructed in the order they are declared inside the class.
Much easier to write in terms of swap. Then you only need to write it once (in the swap method).
Also Best to mark move constructor and move assignment as
noexcept (assuming they are). If you place them in containers it allows them to be moved otherwise containers will attempt to use copy semantics so that they get strong exception guarantees.Pipe(Pipe&& rhs) noexcept
: queue_(nullptr;
, size_(0)
, pop_position_(0)
, queue_offset_(0)
, push_position_(0)
, capacity_(0)
{
rhs.swap(*this);
}
Pipe& operator=(Pipe&& rhs) noexcept
{
rhs.swap(*this);
}
void swap(Pipe& other) noexcept
{
using std::swap;
swap(queue_, other.queue_);
swap(size_, other.size_);
swap(pop_position_, other.pop_position_);
swap(queue_offset_, other.queue_offset_);
swap(push_position_, other.push_position_);
swap(capacity_, other.capacity_);
}You can then write your own swap very easily.
void swap(Pipe& lhs, Pipe& rhs) {
lhs.swap(rhs);
}Variable declaration
Variable declaration order is important.
Objects will always be constructed in the order they are declared inside the class.
Code Snippets
Pipe(Pipe&& rhs) noexcept
: queue_(nullptr;
, size_(0)
, pop_position_(0)
, queue_offset_(0)
, push_position_(0)
, capacity_(0)
{
rhs.swap(*this);
}
Pipe& operator=(Pipe&& rhs) noexcept
{
rhs.swap(*this);
}
void swap(Pipe& other) noexcept
{
using std::swap;
swap(queue_, other.queue_);
swap(size_, other.size_);
swap(pop_position_, other.pop_position_);
swap(queue_offset_, other.queue_offset_);
swap(push_position_, other.push_position_);
swap(capacity_, other.capacity_);
}void swap(Pipe& lhs, Pipe& rhs) {
lhs.swap(rhs);
}Context
StackExchange Code Review Q#103847, answer score: 3
Revisions (0)
No revisions yet.