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

[Florian Hahn via Phabricator via llvm-commits]

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



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

> The pre-merge check has found additional locations where template arguments need to be added. Did you consider using using `void` as default template argument, or make `ThreadPool` a typedef of "ThreadPoolWithResult<void>"?

Thanks for the suggestion! I updated ThreadPool -> ThreadPoolWithResult and add `using ThreadPool = ThreadPoolWithResult<void>`, to avoid adjusting all users.

> However, I don't think that the ThreadPool itself needs to be a template, but just the async implementation such that one can enqueue tasks with different return types to the same thread pool:
>
>   template <typename RetTy>
>   inline std::shared_future<RetTy> async(std::function<RetTy()> F);
>
> AFAIU, `std::packaged_task<>` already handles return values itself.
>
> Edit: I missed that packaged_task itself a template of the return type. However, after calling `get_future`, it it itself packaged into a `std::function<void()>` object that is pushed to the `Tasks` queue, i.e. does not depend on the result type anymore.

I tried using `void` with both `TaskTy` and `PackagedTaskTy`, but both resulted in build errors when sharing the Future.


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/CMakeLists.txt
  llvm/lib/Support/ThreadPool.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114183.388437.patch
Type: text/x-patch
Size: 12789 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211119/217362db/attachment.bin>