我已经在这个线程池上工作了一段时间,以使其尽可能地易于使用。我需要一些有关提高性能的提示,以及一些测试其性能的好方法。我想知道是否有人有任何意见/建议!

这是课程:

#pragma once

#include<thread>
#include<vector>
#include<queue>
#include<mutex>
#include<condition_variable>
#include<functional>
#include<future>

#define MAX_THREADS std::thread::hardware_concurrency() - 1;

//portable way to null the copy and assignment operators
#define NULL_COPY_AND_ASSIGN(T) \
    T(const T& other) {(void)other;} \
    void operator=(const T& other) { (void)other; }

/* ThreadPool class
It is a singleton. To prevent spawning
tons of threads, I made it a singleton */
class ThreadPool{
public:

    //getInstance to allow the second constructor to be called
    static ThreadPool& getInstance(int numThreads){
        static ThreadPool instance(numThreads);

        return instance;
    }

    //add any arg # function to queue
    template <typename Func, typename... Args >
    inline auto push(Func&& f, Args&&... args){

        //get return type of the function
        typedef decltype(f(args...)) retType;

        //package the task
        std::packaged_task<retType()> task(std::move(std::bind(f, args...)));

        // lock jobqueue mutex, add job to the job queue 
        std::unique_lock<std::mutex> lock(JobMutex);

        //get the future from the task before the task is moved into the jobqueue
        std::future<retType> future = task.get_future();

        //place the job into the queue
        JobQueue.emplace( std::make_shared<AnyJob<retType> > (std::move(task)) );

        //notify a thread that there is a new job
        thread.notify_one();

        //return the future for the function so the user can get the return value
        return future;
    }

    /* utility functions will go here*/
    inline void resize(int newTCount){

        int tmp = MAX_THREADS;
        if(newTCount > tmp || newTCount < 1){
            tmp = numThreads;
            numThreads = MAX_THREADS;
            Pool.resize(newTCount);
            for (int i = tmp; i != numThreads; ++i) {
                Pool.emplace_back(std::thread(&ThreadPool::threadManager, this));
                Pool.back().detach();
            }
        }
        else if (newTCount > numThreads) {
            uint8_t tmp = numThreads;
            numThreads = newTCount;
            Pool.resize(numThreads);
            for (int i = tmp; i != numThreads; ++i) {
                Pool.emplace_back(std::thread(&ThreadPool::threadManager, this));
                Pool.back().detach();
            }
        }
        else {
            numThreads = (uint8_t)newTCount;
            Pool.resize(newTCount);
        }


    }

    inline uint8_t getThreadCount(){
        return numThreads;
    }

private:

    //used polymorphism to store any type of function in the job queue
    class Job {
    private:
        std::packaged_task<void()> func;
    public:
        virtual ~Job() {}
        virtual void execute() = 0;
    };

    template <typename RetType>
    class AnyJob : public Job {
    private:
        std::packaged_task<RetType()> func;
    public:
        AnyJob(std::packaged_task<RetType()> func) : func(std::move(func)) {}
        void execute() {
            func();
        }
    }; 
    // end member classes

    //member variables
    uint8_t numThreads; // number of threads in the pool
    std::vector<std::thread> Pool; //the actual thread pool
    std::queue<std::shared_ptr<Job>> JobQueue;
    std::condition_variable thread;// used to notify threads about available jobs
    std::mutex JobMutex; // used to push/pop jobs to/from the queue
    //end member variables

    /* infinite loop function */
    inline void threadManager() {
        while (true) {

            std::unique_lock<std::mutex> lock(JobMutex);
            thread.wait(lock, [this] {return !JobQueue.empty(); });

            //strange bug where it will continue even if the job queue is empty
            if (JobQueue.size() < 1)
                continue;

            (*JobQueue.front()).execute();

            JobQueue.pop();
        }
    }

    /*  Constructors */
    ThreadPool(); //prevent default constructor from being called

    //real constructor that is used
    inline ThreadPool(uint8_t numThreads) : numThreads(numThreads) {
        int tmp = MAX_THREADS;
        if(numThreads > tmp){
            numThreads = tmp;
        }
        Pool.reserve(numThreads);
        for(int i = 0; i != numThreads; ++i){
            Pool.emplace_back(std::thread(&ThreadPool::threadManager, this));
            Pool.back().detach();
        }
    }
    /* end constructors */


NULL_COPY_AND_ASSIGN(ThreadPool);
}; /* end ThreadPool Class */


这是示例用法:

#include "ThreadPool.h"
#include <iostream>

int main(){

    ThreadPool& pool = ThreadPool::getInstance(4); //create pool with 4 threads

    auto testFunc = [](int x){ return x*x; };

    auto returnValue = pool.push(testFunc, 5);

    std::cout << returnValue.get() << std::endl;

    return 0;
}


评论

欢迎使用代码审查!请参阅有人回答时该怎么办。我回滚了Rev 4→2

想要分享要点,以链接到您最终修订的代码吗?

@jb是最终代码github.com/PaulRitaldato1/ThreadPool

@Paul太棒了!感谢您的分享。

#1 楼

有趣的是,Universe是如何工作的-我刚刚完成了自己的线程池实现(尽管在C ++ 17中),它看起来很像您的。我在张贴自己的文章时在首页上发现了这个问题-希望我们俩都走对了!
将副本c'tor和operator =标记为已删除
,而不是实际实现某些您不想使用的东西,在C ++ 11和更高版本中,您可以显式禁止复制构造函数和赋值运算符的调用: :
ThreadPool(const ThreadPool&) = delete;
ThreadPool &operator=(const ThreadPool&) = delete;

将一个函数标记为已删除时,尝试调用它会导致编译时错误。
请勿声明默认构造函数。
上面默认构造函数的声明没有任何作用,除了掩盖了您尝试调用默认ctor时程序无法构建的真正原因。仅当没有明确声明构造函数时,默认构造函数才由编译器定义。
通过声明默认构造函数但不实现它,可以使编译器构建尝试调用它的代码合法-只是发现自己处于链接器将失败的位置,这不是(应该是)格式错误的构建代码的预期结果。
按照9.1删除内联说明符。
6 [dcl.inline]:


在类定义中定义的函数是一个内联函数。 br />摆脱inline

声明Job::func的意义何在,而只用Job::func遮盖它呢?基类成员变量永远不会被触及-它只是在没有任何目的的情况下增加了代码的复杂性。 AnyJob<T>::func的唯一要点是充当不同类型的多态函子(您的Job)的通用基础。使其尽可能简单。
为变量使用好名字,并消除多余的变量
#define DISALLOW_COPY_AND_ASSIGN(T) \
    T(const T&) = delete; \
    T &operator=(const T&) = delete;

如果您的线程向量已经在跟踪该信息,为什么首先要使用AnyJob<T>?为什么numThreadsthread的名称?我希望它是一个线程对象或容器。 condition_variable也不是一个好名字-像Poolworkers这样的名称会更好。
此外,命名约定是什么?您将骆驼盒(threads)和帕斯卡盒(numThreads)混合在一起-这很奇怪。如果您想更进一步,使之更加C ++ ey,则《 C ++核心准则》建议您最好使用下划线。并不意味着要由拥有JobQueue对象的线程调用。为什么要提供它?最好使出错的错误变得困难或不可能(如果在合理范围内)。
鉴于其唯一目的是将工作线程从工作状态转换为空闲状态,反之亦然,这个名字有点奇怪。此外,为什么不简单地将lambda传递给private构造函数呢?这将使您的实现更加简洁。
其他情况
我还将考虑诸如构造函数将线程数不加限制地限制为一组数字的事情-是Funny Behavior™,即程序员应该被警告?也许我们希望程序比处理器具有更多的线程?交给您的一项任务可能会在I / O上等待大量时间-上下文切换可以成为您的朋友!这是很好的代码。我对此不太满意-但这样做的目的是,您可以使代码更好,并希望在此过程中学习一些技巧。干得好!

评论


\ $ \ begingroup \ $
是的,命名约定刚刚下地狱。主要是因为我只是想让所有东西正常工作。不适摆脱numThreads,因为我不确定为什么我把它放在那里。就内联而言,函数位于类定义内,因此我认为它适用于此。而且我的threadManager()函数已经是私有的。感谢您的小费!我会做出您建议的更改!
\ $ \ endgroup \ $
– Paul
19年4月4日在6:31



\ $ \ begingroup \ $
@Paul我对threadManager的错误-它是一个私有声明的类的公共成员,因此很困惑。我对内联声明的观点是,由于所有函数都是在类定义中定义的,因此它们都已隐式声明为内联。但是无论如何,我很高兴能为您提供帮助!你那里有一些不错的代码:)
\ $ \ endgroup \ $
– osuka_
19年4月4日在6:43

#2 楼

@osuka_和@anderas提供了一些非常好的建议。我只想添加几件事:



#define MAX_THREADS std::thread::hardware_concurrency() - 1;


尽管对此进行算术并不是很有意义,这样的宏应该用括号括起来,以便正确执行操作顺序。没有它,您会得到奇怪的东西(假设std::thread::hardware_concurrency()为4):MAX_THREADS * 5 => 4 - 1 * 5 => -1而不是MAX_THREAD * 5 => (4 - 1) * 5 => 15。另外,宏不应以分号结尾。宏的用户应添加分号(就像您一样-int tmp = MAX_THREADS;)。或者,完全避免使用宏(毕竟这是C ++),并使用const auto MAX_THREADS = std::thread::hardware_concurrency() - 1;

push()中:

std::unique_lock<std::mutex> lock(JobMutex);


这是一个小问题,但是看到这一点,我希望在某个时候可以解锁lock(例如threadManager()中的条件变量令人困惑的名称thread)。如果像在这种情况下那样应将锁保持到作用域的尽头,请改用std::lock_guard<std::mutex>

评论


\ $ \ begingroup \ $
hardware_concurrency不是constexpr(编译时间常数),因此您的MAX_THREADS变量应为const。在讨论宏时,应提及宏定义不应以分号结尾。
\ $ \ endgroup \ $
– 1201ProgramAlarm
19年4月4日在14:18

\ $ \ begingroup \ $
@ 1201ProgramAlarm谢谢,已编辑。我什至没有注意到分号!
\ $ \ endgroup \ $
–凯文
19年4月4日在15:05

#3 楼

到目前为止,您已经收到了不错的风格建议。

但是您的游泳池不起作用。因此,让我们尝试解决这个问题。

1。 'resize'既错误又不需要。

如其他人所述,它不是线程安全的。做到这一点极其困难。在实践中,启动后不需要更改线程池的大小。

2。您一次只能使用一个线程。

在threadManager()中,执行带有互斥量的作业。这意味着一次可以执行一个作业,而不必首先创建池。

通过将作业复制到本地变量来修复它,将其从队列中弹出,解锁互斥锁然后才执行它。

3。 shared_ptr比unique_ptr

慢,并且作业队列不需要。但是最好将它们都清除掉,如#6所示。

4。 detach()懒惰且危险

在实践中只有很少的好用,因为它会在程序退出时杀死线程,而不是优雅地等待作业完成。

将其替换为类析构函数中的join()。 (停止使用该单例的另一个原因,其他一些人解释这是不好的。)

您将需要添加其他代码来控制出口:添加一个原子布尔isStopping。
在构造函数的初始化列表中将其初始化为false。
在析构函数上,将其设置为true。
然后在条件变量上调用notify_all()。这样,所有线程都被唤醒,可以测试isStopping的值。
在threadManager()中,在执行作业之前和之后,检查isStopping是否设置为true,并在需要时返回,退出线程。

还需要调整条件变量如果isStopping为true,则返回lambda。


最后,返回析构函数,在所有线程上调用join()。您可以使用两种不同的退出策略:首先执行所有挂起的作业或将其丢弃。丢弃是一个很好的默认设置,因为否则退出将被延迟不确定的时间,直到队列被处理。

5。该singleton

它不仅可以防止您优雅地关闭线程池(因为在退出过程中很晚才调用singleton析构函数,而且可以阻止您的池的真正用例。假设您想要处理两种任务-一种处理速度非常快,一种处理速度很慢,想象一下您在池中排队了许多缓慢的任务,这会使快速的任务等待它们全部执行。

6.您可以用std :: function替换Job和AnyJob,这有两个池,一个用于快速,一个用于慢速,可以分离资源并提供更好的性能。

以及带有lambda的适当的初始化程序,该初始化程序可捕获packaged_task。

7.线程数没有很好的默认值

纯计算像科学模拟一样的负载,在专用服务器上运行将最好与每个内核的线程一起工作(实际上,即使面对这种过度假设,即使是这种基本假设也是错误的叮)。但是,这种情况很少。如果执行I / O,则可以有效地使用比内核更多的线程。如果您对应用程序的不同部分使用多个池(一个用于I / O,一个用于处理),则需要明智地选择它们之间的资源分配。在由多个应用共享的服务器中,您需要牢记其他租户。

我建议完全删除对hardware_concurrency的使用。它邀请用户做出懒惰和糟糕的决定。

评论


\ $ \ begingroup \ $
这些是不使用单例的实际令人信服的论点,我将其考虑在内。我将发布修订后的代码(我实际上发现了由于互斥而一次只使用一个线程的错误)。我想举一个例子说明您在#6中的意思。我发现没有办法将任何类型的打包任务存储为std :: function。我需要packaged_task,因为它们允许使用期货,而std :: function则不允许。我不太确定如何让.join()工作。似乎如果我不加入构造函数,就会出现编译错误。
\ $ \ endgroup \ $
– Paul
19年4月4日在19:41

\ $ \ begingroup \ $
关于原子布尔的建议:在构造函数中将其初始化为false。不要构造它,然后通过为其分配mo_seq_cst进行原子存储。
\ $ \ endgroup \ $
– Peter Cordes
19年6月6日在4:05



\ $ \ begingroup \ $
这里的建议存在两个严重问题:首先,丢弃任务会导致引发异常(因为您要摆脱从未实现的未来)。其次,由于std :: packaged_task无法复制(只能移动),因此无法在此处使用std :: function。该标准要求从Callable和CopyAssignable对象构造std :: function;在这种情况下,任务只是其中之一。
\ $ \ endgroup \ $
– osuka_
19年6月8日在17:40

#4 楼

std :: thread :: hardware_concurrency()可以返回0,您应该处理这种情况。

评论


\ $ \ begingroup \ $
还是1,我认为可能性更大。
\ $ \ endgroup \ $
–val说恢复莫妮卡
19年4月4日在21:03

#5 楼

@osuka_已经提供了详尽的评论,但是我想表明他的评论中缺少的一个重要要点:选择使您的班级成为单身人士的选择以及实现它的方式。您确实有充分的理由使此类成为单身人士。但是有时候,单例模式被认为是反模式,因为它常常使测试更加困难(以及其他缺点)。替代方法是简单地使ThreadPool成为普通类,并使用依赖项注入/求逆技术来定位共享对象。 br />
    //getInstance to allow the second constructor to be called
    static ThreadPool& getInstance(int numThreads){
        static ThreadPool instance(numThreads);

        return instance;
    }


这会在首次使用时构造一个给定值为getInstance的单个实例。问题是:您必须注意


总是使用相同的numThreads调用此方法,或者第一个用法是始终可以完全确定该值正确。

两者都会导致可维护性问题。例如,考虑以下函数:将必须检查numThreads是否也使用prepareWork(),如果使用,则是否通过了正确的线程数。在较大的代码库中,这很容易导致可避免的错误。 br />

评论


\ $ \ begingroup \ $
我知道很多人认为单例是不好的做法。我这样做的原因是由于硬件限制。我不希望用户能够创建许多不同的实例。 100个全部使用最大线程数的线程池实例对我来说似乎是个坏主意,但是我没有那种情况下的经验。它会缓慢爬行直到没有剩余内存或崩溃吗?
\ $ \ endgroup \ $
– Paul
19年4月4日在19:13

\ $ \ begingroup \ $
实现定义的,但是使线程池本身成为单例仍然是错误的处理方式。
\ $ \ endgroup \ $
–没用
19年5月5日在14:59

\ $ \ begingroup \ $
@Paul:如果有那么多线程正在等待I / O(例如,侦听网络套接字),那么没什么大不了的。如果他们都在做不占用太多内存的CPU密集型工作,那么一切都会变慢,但在例如8核系统。您的操作系统的调度时间片足够短,以至于交互式程序偶尔会占用CPU时间。 (并且具有良好的启发式操作系统,可以提高“看起来是交互式的”任务的优先级,例如,不要用尽完整的时间片,这可能不太明显)。如果它们使用大量RAM,则交换空间将受到干扰或至少具有L3缓存。
\ $ \ endgroup \ $
– Peter Cordes
19年6月6日在4:15



#6 楼

还有一个供后代使用...

宏-只需说号即可。

宏的末尾,这意味着您不能写resize(MAX_THREADS)


对宏进行括号括起来,例如

#define MAX_THREADS std::thread::hardware_concurrency() - 1;


so您可以编写resize(MAX_THREADS/2)


不使用宏,我们不再编写K&R C

#define MAX_THREADS (std::thread::hardware_concurrency() - 1)



也不要使用thread::hardware_concurrency()-仍然不能保证您期望的结果:


...该值应仅视为提示。 >


构造函数

unsigned int max_threads() {
    return std::thread::hardware_concurrency() - 1;
}


...不是,不是。就像osuka_所说的那样,只需明确地写出= delete。也可以在这里使用= delete。这清楚地表达了您的意图,您甚至不需要注释,这就是为什么将其添加到语言中。

总是标记单参数构造函数explicit,除非您想从uint8_t进行隐式转换(我很确定您不需要)。并且inline在这里什么也不做。如果出于某种原因,如果您真的想要260个线程池,那就有点尴尬了。

Singletons

尽可能避免单例
如果需要单例(但实际上不需要),请编写std::numeric_limits<uint8_t>::max()包装器,而不是将其烘焙到类中。单例性不是线程池的核心问题,理智的用户可能很合理地希望使用两个线程池,每个线程池具有少量线程,以分离不同类型的任务。您已经允许池大小达到std::numeric_limits<int>::max(),这在法律上可能是巨大的,因此无论如何都无法避免问题。在某个时候,您只需要相信您的用户不会启动每个具有INT_MAX线程的一百万个线程池。
您的Singleton<T>方法将使用hardware_concurrency()-1线程构造单个实例(如上所述,被隐式截断为getInstance)在第一个调用上,但后续调用将完全忽略numThreads参数。这很容易造成混淆,并且容易出错,这强烈建议完全不要耦合实例管理和配置。如果较小,则分离多余的工作线程,但从不告诉它们死亡。任何线程池都应该有一种方法告诉工作线程退出。在uint8_t上使用numThreads方法就足够了,但是您还需要一些方法来告诉virtual bool shutdown()方法退出了哪些线程,以便它可以正确清理池。如果按照另一个答案中的建议完全删除了Job方法,则可以使用简单的resize标志-否则,可以将其大小调整为零并使用相同的关闭通知机制。

您的工作线程不断保持在执行每个任务时使用互斥锁,因此实际上几乎没有并发性。

按住锁的同时从队列中移出当前任务ptr,然后使用作用域解锁器(例如unique_lock,但是完全向后)以在执行互斥体时释放它。



#7 楼

/* ThreadPool class
It is a singleton. To prevent spawning
tons of threads, I made it a singleton */


ThreadPool既可以用作单例,也可以不用作单例。

不需要将ThreadPool实现与“这是一个单例”实现混合在一起。有很多没有必要。在非平凡应用程序中,单例需要注意一些讨厌的事情。

class ThreadPool{
    //add any arg # function to queue
    template <typename Func, typename... Args >
    inline auto push(Func&& f, Args&&... args){
std::thread支持将参数传递给任务,我的经验是不必要的并发症。

只需要一个null函数即可。如果需要,呼叫者可以非常轻松地将其辩论内容打包成lambda。

        //get return type of the function
        typedef decltype(f(args...)) retType;

        //package the task
        std::packaged_task<retType()> task(std::move(std::bind(f, args...)));


为什么这样做的一个例子是一个坏主意。您使用了std::async。如果std::bind已经是f的结果,则此操作与使用参数std::bind调用f的功能不同。 >
        // lock jobqueue mutex, add job to the job queue 
        std::unique_lock<std::mutex> lock(JobMutex);


我建议使用单一责任原则,将您的工作队列从线程池中分离出来。 args...的复杂度的一半是队列,另一半是管理线程。通过将两者拆分,您可以获得两段代码,每段代码的复杂程度降低了一半。

std::bind可以在其他地方重用。 ThreadPool

threadsafe_queue是一个可怕的名字。您不是容器。

template<class T>
struct threadsafe_queue;


如果您知道自己有很多阻塞操作,那么拥有更多线程的硬件并发大小为-1是一件非常明智的事情。

这种逻辑不属于名为inline的类。设置硬限制是不好的。

    /* utility functions will go here*/
    inline void resize(int newTCount){


分离线程是99.999%的时间错误的事情。不要这样穿过主端的线程非常危险且有毒。

        int tmp = MAX_THREADS;


您需要真正弄清楚减少线程数的含义。

                Pool.back().detach();


虽然resize不够用,因为ThreadPool只能移动,所以像这样的类型非常明智和有用。 std::function<void()>packaged_task<R()>。试试吧。

packaged_task<R()>替换packaged_task<void()>。别再把指针弄乱了。

但是,实际上,找到一个仅移动的Job并使用它。如有疑问,请使用值语义。

        else {
            numThreads = (uint8_t)newTCount;
            Pool.resize(newTCount);
        }

std::packaged_task<void()>。但实际上,std::function是多余的; ThreadPool拥有该信息。

也许有第二个“停放线程”向量。

也总是numThreads或任何数据。 br />
Pool.size()的属物。一个将线程安全队列与线程池混合在一起的示例使您感到困惑。 >
    //used polymorphism to store any type of function in the job queue
    class Job {


=0 s设置threadsafe_queue并将线程移动到其他地方以供稍后清理。 up”队列,其他线程甚至可以清理该队列,在该队列中最多保留一个“等待加入”任务。

请注意,我们希望push支持remove_thread-这意味着*kill返回一个threadsafe_queue(增强或不增强)而不是.abort(),因此它可以返回“弹出失败”。另一种选择是它可能会抛出。

如果队列被杀死,所有持久性有机污染物都将中止。

br />
    //member variables
    uint8_t numThreads; // number of threads in the pool
    std::vector<std::thread> Pool; //the actual thread pool


再次使用反模式。

整个正文应显示为:

    std::queue<std::packaged_task<void()>> JobQueue;
    std::condition_variable thread;// used to notify threads about available jobs
    std::mutex JobMutex; // used to push/pop jobs to/from the queue
    //end member variables


干-不要重复自己。应该有一个添加线程的功能,在这里和其他地方都可以使用。

这么累,您将编写一个宏以避免输入宏?

这是一个线程安全队列:在不中断队列的情况下,“放弃所有排队的任务”的功能非常有用。线程池代码也变得更干净。

评论


\ $ \ begingroup \ $
可能只有我一个人,但这听起来像将一个队列完全包裹在另一个类中是一个严重的过大杀伤力。通过在等待任务的实现中添加一行代码,包装线程也无济于事。更重要的是,如果您想返回带有其他T的未来,使用带有T = std :: packaged_task 的队列是尴尬的(不可能吗?)。Job和AnyJob的重点是在其中添加某种类型的擦除这种结构,以便您可以在将来同时返回有意义的值,并存储返回不同类型的任务。
\ $ \ endgroup \ $
– osuka_
19年6月8日在18:07

\ $ \ begingroup \ $
再三考虑:模仿std :: thread和std :: bind是一个好主意,原因有很多:程序员可能对此很熟悉,它更易于使用,并且避免使它们成为#include 进行绑定呼叫。他的方法有许多非常理想的优势,而且成本低廉(因为这样做不会增加太多复杂性)
\ $ \ endgroup \ $
– osuka_
19年6月8日在18:12



\ $ \ begingroup \ $
@osuk没有人应该使用标准绑定。只需使用lambdas。打包的任务void是类型擦除类。它可以存储任何类型R的打包任务R。使用线程调用绑定比使用lambda困难得多,因为传递的参数的r / l值转换非常模糊,以至于msvc多个版本才能正确实现它们。大多数C ++程序员都做得不好。 OP的操作不正确。您可能没有注意到他们做错了。用户可能不知道正确的魔术行为,因此错误和正确的用户都会感到惊讶。
\ $ \ endgroup \ $
– ak牛
19年6月8日在21:00



\ $ \ begingroup \ $
为什么没有人使用绑定?它确实需要使用哪种语言,但是我不明白为什么您应该建议不惜一切代价避免使用它。 Lambda有一些奇怪的地方(例如,移动捕获不可复制的对象会使Lambda无法复制)。如果您了解了移动语义和引用绑定,那么正确实现某件事并不是那么困难。使用packaged_task 的问题在于,您不能接受可返回任何类型的可调用对象(除非在将其添加到队列之前发生某种类型的擦除)。
\ $ \ endgroup \ $
– osuka_
19年6月8日在21:39

\ $ \ begingroup \ $
@ Yakk,std :: packaged_task foo; std :: packaged_task bar = std :: move(foo);不编译,但std :: packaged_task foo; std :: packaged_task bar(std :: move(foo));做。
\ $ \ endgroup \ $
– Evg
19年6月28日在21:24

#8 楼

这里已经有很多不错的评论,总体来说还不错... ...

resize函数不是线程安全的,尤其是由于您使用的是单例,因此您不知道从何处调用它,不使该线程安全会使您处于竞争状态。

您没有在通知之前调用JobQueue,这似乎很奇怪,被唤醒的线程至少必须等待push调用才能释放锁。

虽然绝对应该考虑核心数量,但是实际上,线程的运行时行为通过创建您要限制的人为限制来确定线程池的最佳大小。使用您的游泳池。我将使用std::thread::hardware_concurrency()作为构造函数的默认值,但不限制池的大小。现在,任何使用池大小的人都可以确定其用例的最佳大小。

关于单身人士,我必须投入两分钱,我在代码中的某些地方使用单身人士,但还没有找到限制单例对象构造的充分理由,通过允许单例访问或对象的新构造,您可以创建多种用途的机会,这使测试变得更加容易,并且有时使您摆脱单例访问。只要让构造函数公开,任何看着您的班级的人都可以看到啊,我可以使用单例,或者我可以创建一个并传递它。

评论


\ $ \ begingroup \ $
总体而言,好的建议-但是根据定义,可以实例化多次的单例不是单例。如果您可以“使用单例或创建一个并传递它”,则它不是单例-只是全局的。
\ $ \ endgroup \ $
– osuka_
19年4月4日在17:57

\ $ \ begingroup \ $
谢谢!我通过锁定和解锁解决了这个问题。性能分析表明,这实际上是在降低性能,因为threadManager(因为已删除代替了lambda)在执行作业时一直保持锁定,从而阻止了其他所有线程访问队列。使用更新后的代码进行修改
\ $ \ endgroup \ $
– Paul
19年4月4日在19:34