mtrebi/thread-pool

Thread pool implementation using c++11 threads

thread-pool's Issues

应用层，类的非静态函数不能使用，目前仅静态函数可用

测试例程如下：
class A {
public:
static int Afun(int n = 0) {
std::cout << n << " hello, Afun ! " << std::this_thread::get_id() << std::endl;
return n;
}

void Cfun(void) {
    std::cout <<  "  hello, Cfun !  "<< "mem_a :"<<mem_a <<"  " << std::this_thread::get_id() << std::endl;
    return ;
}

private:
int mem_a=0;
};
void test_thread_pool()
{
// Create pool with 3 threads,max_thr_num:100
//同时存活的线程的最小数量3,同时存活的线程的最大数量100
ThreadPool pool(1,2);

// Initialize pool
pool.init();

std::cout << "main thread id :"<<std::this_thread::get_id()<< std::endl;

 std::future<int> gg = pool.submit(A::Afun, 9999);//静态函数成功
 A A_obj;
std::future<int> hh = pool.submit(A_obj.Cfun,); //非静态函数不成功

}

: error: invalid use of non-static member function
std::future hh = pool.submit(A_obj.Cfun);
^

duplicated mutex?

Great implementation of thread pool! It is short and clear.

I have a question:

There is "m_conditional_mutex" in class ThreadPool
And there is "m_mutex" in class SafeQueue

Do these two mutex serve the same purpose i.e. to lock the queue? Queue is the shared resource that everybody try to get hold of. So we need a lock. But one lock is enough.

Another potential issue is that, "m_shutdown" in class ThreadPool is also a shared resource. It should have a lock associated with it.

These are the only two shared resources that existed in the system. Therefore, two locks are required in total.

How to wait for all tasks to finish?

Is there a best way to solve this problem?

Not threadsafe

thread-pool/include/ThreadPool.h

Line 27 in dac6bdd

while (!m_pool->m_shutdown) {

Bool isn't atomic or use lock.

Is there a way to have a member method as the executed function?

Hi,

I assume the answer is "no" of course, but, before I make that conclusion, I would prefer asking in here :). Can I somehow have a member-method (non-static) be my executed function when submitting?

Thanks!

Building problem - cmake configure

cmake configure fails with the following error:

CMake Error at CMakeLists.txt:23 (add_executable):
  Cannot find source file:

    src/main.cpp

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
  .hpp .hxx .in .txx


CMake Error at CMakeLists.txt:23 (add_executable):
  No SOURCES given to target: main


CMake Generate step failed.  Build files cannot be regenerated correctly.

The output file doesn't output anything on compiling on Windows with MinGW-w64

Let me first say that I'm a complete newbie to everything related to C++.

I've tried compiling the example code on Windows using MinGW-w64. It compiles without throwing an error, but the executable doesn't output anything to the console upon running. Below in the complete log. Please help me figure out the problem, and if this is the expected output, help me understand how to wait for the thread pool to finish its jobs.

F:\Programming\C++\thread-pool>mkdir build 

F:\Programming\C++\thread-pool>cd build

F:\Programming\C++\thread-pool\build>cmake .. -G "MinGW Makefiles"
-- The C compiler identification is GNU 8.1.0
-- The CXX compiler identification is GNU 8.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/ProgramData/chocolatey/bin/gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/ProgramData/chocolatey/bin/g++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: F:/Programming/C++/thread-pool/build

F:\Programming\C++\thread-pool\build>make
Scanning dependencies of target main
[ 50%] Building CXX object CMakeFiles/main.dir/example/main.cpp.obj
[100%] Linking CXX executable main.exe
[100%] Built target main

F:\Programming\C++\thread-pool\build>main

F:\Programming\C++\thread-pool\build>

the runtime is very slow when you have a lot of light functions

first of all thank you for code sharing,
Can you help me please,
I tested your code in my projet, but i found the runtime is very slow, compared to the normal code (without threadpool) or compared normal thread. For exmeple my code like this:
//////////////////////////////////////////////////
void myfunction(int k){
std::vector myvetcor;
myvector.push_back (k*2.77+25/5);
}
int main()
{

 {
  ThreadPool pool{ 8};
   for (int i = 0; i < 20000; ++i)
		{
			pool.enqueue([ i]() {
                                                myfunction(i);
			});
		}
	}

return 0;
}
////////////////////////////////////////////////////
thank you

Author of another thread pool says mtrebi's thread pool fails tests

From:
https://github.com/Fdhvdu/ThreadPool/tree/master/comparison

First, I will not compare nbsdx, philipphenkel, tghosgor and mtrebi. Because their works has potential bug.

In the mtrebi test directory:
https://github.com/Fdhvdu/ThreadPool/tree/master/comparison/mtrebi

Warning
Do not use this.
It gets stuck when runing out_100 sometimes.

Synchronization on shutdown.

I have noticed that there seems to be a race condition in the shutdown process of ThreadPool. By example let Thread1 be the main thread executing shutdown() and let Thread2 be a ThreadWorker executing operator(). If we take the following interleaving of calls (assuming that the queue is empty by the start):

Thread2: checks the while condition and returns true.
Thread1: sets m_shutdown to false.
Thread1: calls m_conditional_lock.notify_all().
Thread2: takes ownership of m_pool->m_conditional_mutex.
Thread2: call m_pool->m_conditional_lock.wait( lock ).

In the above scenario it can be seen that the notify_all() is completed before Thread2 calls wait which means that the ThreadWorker remains waiting indefinitely (unless a spurious wake-up occurs) and so the call to join() in shutdown() can't be completed stopping the shutdown of the thread pool. One suggestion to fix this would be to provide a predicate to the wait(lock), such as m_pool->m_conditional_lock.wait( lock , [this]{ return (!this->m_pool->m_queue.empty()) || this->m_pool->m_shutdown;});

Would you tell me why you use notify_one instead of notify_all here?

thread-pool/include/ThreadPool.h

Line 94 in 99ae49b

m_conditional_lock.notify_one();

I am reading the code and try to learn from it, and I am puzzled that you use notify_one in this line rather than notify_all, would you please tell me why notify_one is better here ?

Bug report: When sub-task are performed quickly, the results are inaccurate

#include <iostream>

#include "../include/ThreadPool.h"

class TEST {
private:
  std::mutex mtx;
  std::vector<int> v1[2];
  std::priority_queue<int> q;
public:
  void init() {
    v1[0].push_back(1);
    v1[0].push_back(2);
    v1[0].push_back(3);

    v1[1].push_back(1);
    v1[1].push_back(3);
    v1[1].push_back(2);
  }
  
  void push(const int& index) {
    for (auto& i : v1[index]) {
      std::lock_guard<std::mutex> m{mtx};
      q.push(i);
    }
  }

  void run() {
    ThreadPool pool(2);
    pool.init();
    // https://github.com/mtrebi/thread-pool/issues/14
    // https://github.com/mtrebi/thread-pool/issues/7
    for (int i = 0; i < 100; i++) {
      for (int j = 0; j < 2; j++) {
        auto func = std::bind(&TEST::push, this, j);
        pool.submit(func);
      }
    }
    pool.shutdown();

    int tmp{-2};
    std::cout << q.size() << std::endl;
  }
};

int main(int argc, char *argv[]) {
  TEST t;
  t.init();
  t.run();
  return 0;
}

The output of the above code is not accurate.

315 // sometimes
291 // sometimes
69 // sometimes

Is it redundant with two locks?

Hey,bro
I have a question to ask for your help. I found your SafeQueue having a lock while ThreadPool also having a lock. Is they redundant？
I think, in your operator(),
If reserve SafeQueue，change the code to below:

void operator()()
{
	std::function<void()> func;
	bool dequeued;
	while (!m_pool->m_shutdown)
	{
		{
			if (m_pool->m_queue.empty())
			{
				std::unique_lock<std::mutex> lock(m_pool->m_conditional_mutex);
				m_pool->m_conditional_lock.wait(lock);
			}
			dequeued = m_pool->m_queue.dequeue(func);
		}
		if (dequeued)
		{
			func();
		}
	}
}

if use normal queue(without lock), operator() stay just same as your previous version
Please point out my mistake if i was wrong, thanks！

Heap allocation issues

I am writing a fast SDK for executing tasks and executing callbacks registered to events. I used your approach for execution and noticed that heap allocation is done whenever a std::function is created which is suboptimal for performance. Have you considered other alternatives?

How to use C++ member function in thread pool

First of all thanks for your detailed write up to explain the code!

I am trying to use the threadpool submit function to take a C++ class member function. I am not able to compile. I followed standard std::thread submitting a class member function method. Like the code shown below.

class A
{
   int i;
public:
      A() { i =10; };
    ~A()
    void func1() { i++;};
};

A objA;

ThreadPool tPool(10);

tPool.submit(&A::func1,&objA);

But I am getting the following error.
error: no matching function for call to ‘ThreadPool::submit(void (A::)(), A)’

Does the current submit() template function accept class member function? If not can you provide some tip as to how this function can be modified to accept class member function?

Appreciate your help.

Here is the std::thread method of passing a non static member function.

class Task
{
public:
	void execute(std::string command);
};

Task * taskPtr = new Task();

// Create a thread using member function
std::thread th(&Task::execute, taskPtr, "Sample Task");

Thanks,
Raj