Lockfree ThreadPool implementation

Question

How could this template class ThreadPool be improved?

I use Boost queue to keep std::function's numbers from std::array(simple hashing) from which I can later retrieve the std::function's objects to execute them with operator(). It's not possible to store them directly in boost::lockfree::queue (Stored Object Type must have non-trivial destructor).

That's why I emulate primitive lockfree container with std::array<std::pair<int , ...>. The GuarderArray template class lacks garbage collector.

#include <iostream>
#include <thread>
#include <mutex>
#include <memory>
#include <atomic>
#include <functional>
#include <boost/lockfree/queue.hpp>
#include <array>


using std::cout ; using std::cin; using std::endl;
using std::mutex; using std::atomic;
using std::array; using std::function;

template <int NN> using guardedArray =  std::array<std::pair<std::atomic<bool>,std::function<void()>>, NN> ;

template <typename T> using lockfreequeue = boost::lockfree::queue<T,boost::lockfree::capacity<1000>>;

class Functor{
    int myNN;
public:
    Functor(int nn):myNN(nn){};
    Functor(std::function<void()> ff) : myff(ff){};
    std::function<void()> myff;
    Functor(const Functor&) = default;
    Functor operator=(const Functor&) ;
    ~Functor() = default;
    void operator()(){
    myff();

    }
};



template <int NN > class GuardedArray{
    guardedArray<NN> myGuardedArray; 
public:
    int insertObject(std::function<void()> ff){
    int kk ;
    bool empty;

    do{
    kk = rand () % NN;
    empty = myGuardedArray[kk].first;
    cout << "spining ... " << endl;


    }while(!(std::atomic_compare_exchange_weak(&myGuardedArray[kk].first,&empty,true)));
    myGuardedArray[kk].second = ff;
    return kk;
    };
    std::function<void()> getIndex(int ii){return myGuardedArray[ii].second  ; };
    std::thread garbageCollectorThread;



};

template <int NN > class ThreadPool{

private:
    std::thread tt[NN];
    GuardedArray<1000> myGA;
    bool endFlag;
public:
    ThreadPool():endFlag(false){

    for(int ii = 0 ; ii < NN ; ++ii){       
        std::thread localThread(&ThreadPool<NN>:: threadLoop, this);
        tt[ii].swap(localThread);

    }

    };
    void bookTask(std::function<void()> ff){
    int ii = myGA.insertObject(ff);
    mySync.push(ii);
    };

    bool demandedAccess[NN];
    int waitingProcess;
    ~ThreadPool() {endFlag = true; for (int ii = 0 ; ii < 10 ; ii++  ){  tt[ii].join();}   };
    int myWrapper();
    void threadLoop();
    lockfreequeue<int> mySync;

};

template <int NN> void ThreadPool<NN>::threadLoop(){
    std::thread::id kk  = std::this_thread::get_id();
    std::function<void()> fun ; 
    int ff;
    while(!endFlag){

    if(!mySync.empty()){        
        bool result = mySync.pop(ff);
        if(result){
        fun = myGA.getIndex(ff);        
        fun();
        }
    }
    }
}


int main(int argc, char** argv){

    ThreadPool<10> tp;
    Functor ff(( [](){ cout <<"a kuku  "<<endl;  }));
    tp.bookTask(ff);
    std::this_thread::sleep_for(std::chrono::seconds(20));
    return 0 ; 
}

Answer 1

Just some stylistic things that stick out to me:

• I think it's a bit cumbersome to have all of this:

  using std::cout ; using std::cin; using std::endl;
  using std::mutex; using std::atomic;
  using std::array; using std::function;
  

  You're also still prefixing std:: in places, which defeats the purpose of having this. Since you could always end up using something not already in this list, you might as well keep prefixing std:: as you're doing already, and remove all of that above.

• Some of your indentation and semicolon placing are inconsistent. In some places you indent or not, and in some places you have a space before the ending semicolon. Keep the indentation consistent (you should utilize it) and choose what to do with all of your semicolons.

• It would be more readable and maintainable to place the template statements on separate lines from the class or function, especially if either statement is lengthy:

  template <typename T>
  class Class
  {
  };
  

  template <typename T>
  void function()
  {
  }