我为相当简单的多线程应用程序编写了一个小而简单的ThreadPool类。
Threadpool类使用实际的线程管理向量。Thread结构用于存储线程的函数。ThreadPoolElement跟踪线程及其状态。#pragma once
#include <vector>
#include <thread>
#include <mutex>
namespace frm { namespace util {
template <typename F, typename ... Args>
struct Thread {
Thread(F&& f)
: func(f) {
}
std::function<void(Args...)> func;
};
class Threadpool {
public:
Threadpool(const size_t& capasity = std::thread::hardware_concurrency())
: m_capacity(capasity) {
m_threads.reserve(m_capacity);
}
virtual ~Threadpool() {
for (auto& t : m_threads) {
t.thread.join();
}
}
template <typename Thread, typename ... Args>
void getThread(Thread& t, Args... args) {
while (m_threads.size() >= m_capacity) {
cleanup();
}
std::lock_guard<std::mutex> lock(m_mutex);
m_threads.emplace_back(ThreadPoolElement());
ThreadPoolElement& elem = m_threads[m_threads.size() - 1];
elem.thread = std::thread([&] (ThreadPoolElement* e, Thread& t, Args... args) {
t.func(args...);
e->is_done = true;
}, &elem, t, args...);
}
size_t activeThreads() const {
return m_threads.size();
}
size_t reservedThreads() const {
return m_capacity;
}
void cleanup() {
for (int i = 0; i < m_threads.size(); i++) {
if (m_threads[i].is_done) {
std::lock_guard<std::mutex> lock(m_mutex);
m_threads[i].thread.join();
m_threads.erase(m_threads.begin() + i--);
}
}
}
private:
struct ThreadPoolElement {
std::thread thread;
volatile bool is_done = false;
};
size_t m_capacity;
std::mutex m_mutex;
std::vector<ThreadPoolElement> m_threads;
};
} }我的问题很简单:
发布于 2019-09-25 09:48:30
我发现了一些问题,它们在这里(按照发现它们的顺序)。清单看起来可能很长,但这并不意味着你做得很糟糕。相反,这意味着反馈是详细的;)
如果某个问题以主修开头,这意味着它将阻止我使用您的类。
Thread结构实际上不是一个线程。实际上,它只存储一个std::function成员。你为什么这么说?Threadpool有一个虚拟析构函数。这是不必要的,因为从它继承是没有意义的。我建议你把它变成非虚拟的。get通常表示您从它返回一些东西。但是,getThread不返回任何内容。即使t是通过引用传递的,您也不会更改它。也许addTask是个更好的名字。getThread,直到其中一个线程空闲为止。这很糟糕。此外,您还使用一个活动循环来阻止,这意味着等待占用一个CPU核心的100%。这真的很糟糕。线程和线程池的全部要点是用户不需要等待。我建议您将任务(您的std::function)复制或移动到池中,并在某个线程空闲时立即执行它。这样,getThread就可以立即返回。std::thread。但是,创建线程有一些开销,如果您有很多小任务,这将产生很大的不同。我建议您创建从某个容器获取任务的m_capacity工作线程。这将您的问题转换为典型的生产者-消费者问题(添加任务是生成工作,然后由线程使用)。std::function<void(Args...)>存储单个任务。这意味着线程池不支持返回值。一个常见的模式是addTask返回一个std::future,它将在某个线程完成任务后接收返回值。此外,线程池用户可以使用std::future来检查任务是否完成,并且可以使用它等待任务完成。getThread函数使用t的引用。这意味着,如果t在线程仍在运行时被破坏,就会出现问题。这很糟糕,因为t是由用户传递的,因此池在其生存期内没有控制权。不存在的文档应该是一个很好的地方,可以提到在任务完成之前不能销毁t :)通常情况下,您将std::move任务,它将控制转移到池。m_mutex,它会保护m_threads不受比赛条件的影响。这意味着,无论何时使用m_threads,都希望锁定互斥锁。由于在另一个线程修改变量时读取变量是未定义的行为,因此需要将互斥锁在更多的地方。在m_thread或其元素的一些用法中,互斥锁不被锁定(我不是指构造函数/析构函数)。ThreadPoolElement有一个volatile bool。根据编译器和平台的不同,volatile可以做您认为它所做的事情。但是,你不能依赖这一点。您应该使用std::atomic<bool>代替。Threadpool有一个小写的p,但是ThreadPoolElement有一个大写的P。我建议您重构Threadpool,这样就不需要为每个任务创建一个新的std::thread对象。通常,线程池有一个具有挂起任务的队列和固定数量的工作线程。工作线程从队列中抓取任务并完成它们。这样,就可以在不需要创建新线程的情况下添加许多任务。
您可能需要查看std::packaged_task和std::future类,因为它们可以简化您的实现。您可能可以摆脱Thread和ThreadPoolElement类。
https://codereview.stackexchange.com/questions/229613
复制相似问题