HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Lockless SCSP queue

Submitted by: @import:stackexchange-codereview··
0
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);

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 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.