progschj / ThreadPool

A simple C++11 Thread Pool implementation
zlib License
7.64k stars 2.21k forks source link

using previous example code breaks #1

Closed jarrettchisholm closed 11 years ago

jarrettchisholm commented 11 years ago

using the 'usage' code from your previous blog post doesn't compile:

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

int main() {
    // create a thread pool of 4 worker threads
    ThreadPool pool(4);

    // queue a bunch of "work items"
    for(int i = 0;i<8;++i)
    {
        pool.enqueue([i]
        {
            std::cout << "hello " << i << std::endl;

            std::cout << "world " << i << std::endl;
        });
    }
}

errors out with:

main.cpp: In function 'int main()':
main.cpp:17:7: error: no matching function for call to 'ThreadPool::enqueue(main()::<lambda()>)'
main.cpp:17:7: note: candidate is:
In file included from main.cpp:1:0:
ThreadPool.h:117:15: note: template<class T, class F> Result<T> ThreadPool::enqueue(F)
ThreadPool.h:117:15: note:   template argument deduction/substitution failed:
main.cpp:17:7: note:   couldn't deduce template parameter 'T'
scons: *** [build/main.o] Error 1
jarrettchisholm commented 11 years ago

Needed to specify what T was. After setting it to <void> it works....the code that works:

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

int main() {
    // create a thread pool of 4 worker threads
    ThreadPool pool(4);

    // queue a bunch of "work items"
    for(int i = 0; i < 8; ++i) {
        pool.enqueue<void>([i] {
            std::cout << "hello " << i << std::endl;

            std::cout << "world " << i << std::endl;
        });
    }
}
progschj commented 11 years ago

Yes I'm currently thinking about adding an example to the repository. This version here also allows return values and waiting on the result (somewhat similar to std::future).

#include <iostream>
#include <vector>
#include <algorithm>
#include <iterator>
#include "ThreadPool.h"

int main()
{
    ThreadPool pool(4);
    std::vector< Result<int> > results;

    for(int i = 0; i < 8; ++i) {
        results.push_back(
            pool.enqueue<int>([i] {
                std::cout << "hello " << i << std::endl;

                std::cout << "world " << i << std::endl;
                return i*i;
            })
        );
    }   

    for(size_t i = 0;i<results.size();++i)
        std::cout << results[i].get() << ' ';
    std::cout << std::endl;

    return 0;
}
jarrettchisholm commented 11 years ago

ooh cool - I was thinking that was what the Result class was for.

Out of curiousity, why not just use the std::future instead of making your own class? (forgive me if this is a silly question, I'm still learning the threading/c++ ropes)

progschj commented 11 years ago

Not silly at all. One of the reasons for writing this was to understand the internals of a thread pool/futures better. So launching std::packaged_tasks in separate threads and using the future you get from that is probably better. Using std::promise would also be a way to to "return" a value from a thread without requiring this custom Result construct. I'm not really done with understanding all the new toys in the standard either ;).

progschj commented 11 years ago

I moved the old version to a "legacy" subdirectory and rewrote it so it uses std::future instead of the custom Result type. That simplifies the implementation and future is probably higher quality than my homebrew Result type anyway.

progschj commented 11 years ago

There are also examples now for both implementations so I'll close this issue for now.

jarrettchisholm commented 11 years ago

cool cool :) thanks for helping me out :)