[PATCH] D114183: [ThreadPool] Add template argument for future result type.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 20 10:06:26 PST 2021


fhahn updated this revision to Diff 388709.
fhahn added a comment.

Updated so there's no need for adding a template argument to ThreadPool.

In D114183#3144443 <https://reviews.llvm.org/D114183#3144443>, @Meinersbur wrote:

> More crucially, the line
>
>   Tasks.push(std::move(PackagedTask));
>
> wraps the PackagedTask into <https://en.cppreference.com/w/cpp/thread/packaged_task/operator()> a `std::function<void()>` which does not depend on the return type anymore. That is, we don't need `ThreadPool::Tasks` to be a template of the return type, i.e. it can store tasks of any return type (I think the actually result value is stored as a closure parameter of the `std::function<void()>` object, the `std::future` can be used to retrieve it), the ThreadPool itself does not need to store the future which was only returned by `asyncImpl` for the caller to remember.

Thanks! It looks like it is possible to first construct a std::packaged_task<RetTy()>, extract the future and then move the object into a std::packaged_task<void()>.

It required a few changes, but it looks like it is working now without the extra template argument. It also seems `async` also needs to convert it's argument to a std::function object before calling `asyncImpl`, so `asyncImpl` can determine the correct template argument types for `std::function`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114183/new/

https://reviews.llvm.org/D114183

Files:
  llvm/include/llvm/Support/ThreadPool.h
  llvm/lib/Support/ThreadPool.cpp
  llvm/unittests/Support/ThreadPool.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114183.388709.patch
Type: text/x-patch
Size: 5431 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211120/12e9297f/attachment.bin>


More information about the llvm-commits mailing list