These are obviously major breaking changes. However, they make the library much easier to install and to use.

I needed when_all and then continuation support (only available in boost::future at the moment), so I added optional boost::future support.

I also didn't like the fact that the names and header-guards were global, as they may collide with other names. Everything is now in namespace tp and the header-guards are prepended with THREAD_POOL_.

Thanks Vittorio, I see you really use this piece of code. I'm going to add you as one of co-owners

Thank you. Notice that just yesterday I removed the process functionality from the thread pool in commit 10d27ac, as I believed it was "outside of the scope" of the project.

You may want to revert to 06d600a in order to keep the API more stable for the users until we decide if process should be part of the library or not.

No problem with that so far. If someone needs process he/she let you know =)

thread-pool-cpp has been added to the Hunter package manager.

@SuperV1234 : I will update the package with these improvements.

Hunter is pure CMake, but does require some additional syntax in CMake to make it all work in the package manager as well as a package config installation step. Those changes can be disabled by default.

@inkooboo : Let me know if you would like to host these changes upstream/here, and I can submit a PR.

Some sample syntax is shown below:

CMakeLists.txt:

hunter_add_package(thread-pool-cpp)
find_package(thread-pool-cpp CONFIG REQUIRED)
target_link_libraries(... thread-pool-cpp::thread-pool-cpp)

Version can optionally be specified in the user's project:

${CMAKE_SOURCE_DIR}/cmake/Hunter/config.cmake:

hunter_config(thread-pool-cpp VERSION 1.0.0-p2)

@headupinclouds Interesting. Do you mean changes in thread-pool-cpp repo itself? If so, would you please submit PR, so I can look at them.

Ok, great. I will summarize some small changes here, and follow up with a PR (probably this weekend). Actually, I just reviewed the mods, and since thread-pool-cpp has no library dependencies, there is no Hunter specific syntax required. The only CMakeLists.txt change is a package config installation.

Here are some non Hunter specific changes in that fork, which have allowed me to use this effectively on C++11 platforms that do support lambda initialization extensions, but don't contain true thread_local support (backwards compatibility requirements for existing releases, etc).

https://github.com/hunter-packages/thread-pool-cpp/blob/hunter.develop/thread_pool/worker.hpp#L4-L33

1. Top level CMake THREAD_POOL_HAS_THREAD_LOCAL_STORAGE compiler check for true thread_local support, with various fallbacks

https://github.com/hunter-packages/thread-pool-cpp/blob/hunter.develop/CMakeLists.txt#L16-L29

# If true C++11 thread_local support exists, we will use it:
include(thread_pool_has_thread_local_storage)
thread_pool_has_thread_local_storage(THREAD_POOL_HAS_THREAD_LOCAL_STORAGE)
message("THREAD_POOL_HAS_THREAD_LOCAL_STORAGE: ${THREAD_POOL_HAS_THREAD_LOCAL_STORAGE}")

if(NOT THREAD_POOL_HAS_THREAD_LOCAL_STORAGE)
  # Else, we will check for backups
  include(thread_pool_has_thread_storage)
  thread_pool_has_thread_storage(THREAD_POOL_HAS_THREAD_STORAGE)
  message("THREAD_POOL_HAS_THREAD_STORAGE: ${THREAD_POOL_HAS_THREAD_STORAGE}")
  if(NOT THREAD_POOL_HAS_THREAD_STORAGE)
    message(FATAL_ERROR "Compiler does not support: thread_local, __thread, or declspec(thread)")
  endif()
endif()

#if THREAD_POOL_HAS_THREAD_LOCAL_STORAGE
    #define ATTRIBUTE_TLS thread_local
#elif defined (__GNUC__)
    #define ATTRIBUTE_TLS __thread
#elif defined (_MSC_VER)
    #define ATTRIBUTE_TLS __declspec(thread)
#else // !__GNUC__ && !_MSC_VER
    #error "Define a thread local storage qualifier for your compiler/platform!"
#endif

2. Worker template STORAGE_SIZE

template <size_t STORAGE_SIZE=128>
class Worker {
public:

3. Package config install:
   https://github.com/hunter-packages/thread-pool-cpp/blob/hunter.develop/CMakeLists.txt#L46-L96

install(
  EXPORT "${targets_export_name}"
  NAMESPACE "${namespace}"
  DESTINATION "${config_install_dir}"
  )

There were some additional path/installation changes, which I believe this PR addresses.

@headupinclouds All of above looks reasonable.

1. I would take name TASK_SIZE instead of STORAGE_SIZE for Worker's template variable.
2. Correct me if I'm wrong here. If we merge hunter-specific CMake, it is still possible to build and run tests/benchmark without hunter environment/dependency. Right?

Correct me if I'm wrong here. If we merge hunter-specific CMake, it is still possible to build and run tests/benchmark without hunter environment/dependency. Right?

Yes.

Also, since this is a header only library without additional dependencies, I realized it can be run without any hunter specific syntax -- just a CMake package config installation step to make it work with standard find_package() syntax:

find_package(thread-pool-cpp CONFIG REQUIRED)
target_link_libraries(... thread-pool-cpp::thread-pool-cpp)

This is because thread-pool-cpp has no dependencies, and doesn't require any package management from hunter itself, it just needs to provide standard CMake hooks to allow it to work as a package.

The benchmarks that use boost, for example, could use hunter to manage that dependency portably . That type of change would require a small mod to the CMakeLists.txt managing the tests. See the HunterGate() description here:

Where HunterGate.cmake is local CMake module:

HunterGate(
    URL "https://github.com/ruslo/hunter/archive/v0.7.0.tar.gz"
    SHA1 "e730118c7ec65126398f8d4f09daf9366791ede0"
)

https://github.com/hunter-packages/gate

I could submit two independent PR's for review if you like:

1. general changes + cmake package install (required)
2. huntergate() use for building tests with boost (nice to have)

@headupinclouds Sounds good. Thank you! Actually only benchmark itself depends on boost and this dependency is also optionally turned on/off as preprocessor definition. Probably it is good idea to remove this boost part completely with dependency itself. Up to you.

Why was the default template parameter added to the ThreadPool class in this PR?

    template <typename TSettings = void>
    class ThreadPool

@headupinclouds: I was cleaning up the implementation and accidentally committed my WIP changes to the master branch of my fork, which then got merged. I will finish cleaning it up today (removing the template parameter completely).

@inkooboo : Ok, thanks for the clarification. I'll be working on a PR in parallel. I can remove the TSettings class template parameter, and will replace this with a TASK_SIZE parameter based on the discussion above. Something like this:

template <size_t TASK_SIZE=128>
class ThreadPool 
{
public:
    using FixedWorker = Worker<TASK_SIZE>;
    //...
};

template <size_t TASK_SIZE=128>
class ThreadPool { ... };

I can take care of the template change as part of this PR.

@SuperV1234 , @inkooboo ☝️ I'll submit this for review as discussed.

No problem with that so far. If someone needs process he/she let you know =)

I am using process() in an existing project :). The syntax changes are fine, and I'd like to stay in sync with upstream, but this is a breaking design change for me. This seems to be a useful feature for a general purpose thread pool.

@inkooboo , @SuperV1234 : Thoughts?

@headupinclouds: if you're going down that route for TASK_SIZE, consider wrapping it inside a ThreadPoolSettings struct and templating on that instead. This makes the API of ThreadPool stable if we decide to add a new "configuration parameter" in the future (e.g. compile-time strategy to choose number of workers).

template <size_t TASK_SIZE=128> struct Settings
{
    static constexpr auto task_size = TASK_SIZE;
};

template <typename TSettings> class ThreadPool;

@headupinclouds: what I dislike about process is that it's a very heavyweight technique which sort of defeats the purpose of the thread pool. It requires the instantiation of a packaged_task, which could potentially incur a dynamic allocation.

I'm confident that there's a nicer abstraction that allows stack-based semantics which works well with a thread pool when you are aware of the lifetime of the values you're expecting (I was experimenting with something like that here). Nevertheless, if you send a PR putting process back in I would accept it, as I didn't intend to break the API - we can then discuss whether or not process should be part of this library later.

I'm confident that there's a nicer abstraction that allows stack-based semantics which works well with a thread pool when you are aware of the lifetime of the values you're expecting (I was experimenting with something like that here). Nevertheless, if you send a PR putting process back in I would accept it, as I didn't intend to break the API - we can then discuss whether or not process should be part of this library later.

Ok. I'll take a look at wrapping TASK_SIZE, and will add process() back for now. I'm open to migrating to a suitable alternative in the future if you contribute something along these lines -- although both implementations could probably live together. If packaged_task is deemed to be redundant, we could mark it as deprecated and phase it out over time.

I'll be adding a few things, such as explicit versioning and build tests to help with package management in the resulting PR. We can discuss it there.