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 ; 
}

• Loki Astari: Can you please at least add good indentation. Currently its nearly impossible to read.
• user40334: Seems everyone has his own >>good<< indentation language...
• Morwenn: @user40334 On Code Review, you should not modify your code once posted. Otherwise, it invalidates the answers.
• Loki Astari: @user40334: There are a couple of acceptable indention patterns. Yours matches none of them.
• Jamal: There were still some changes that invalidated my answer. You may still add the additional functionality that was not addressed, otherwise please keep everything else intact.

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()
  {
  }
  

• user40334: yeah code paste was little messy , I better auto-format this in editor. My initial bet was to use the std::move casting to prevent spourious cp ctors.