[llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 26 20:20:30 PST 2024
================
@@ -92,30 +104,32 @@ class ThreadPool {
&Group);
}
- /// Blocking wait for all the threads to complete and the queue to be empty.
- /// It is an error to try to add new tasks while blocking on this call.
- /// Calling wait() from a task would deadlock waiting for itself.
- void wait();
+private:
+ /// Asynchronous submission of a task to the pool. The returned future can be
+ /// used to wait for the task to finish and is *non-blocking* on destruction.
+ template <typename ResTy>
+ std::shared_future<ResTy> asyncImpl(std::function<ResTy()> Task,
+ ThreadPoolTaskGroup *Group) {
- /// Blocking wait for only all the threads in the given group to complete.
- /// It is possible to wait even inside a task, but waiting (directly or
- /// indirectly) on itself will deadlock. If called from a task running on a
- /// worker thread, the call may process pending tasks while waiting in order
- /// not to waste the thread.
- void wait(ThreadPoolTaskGroup &Group);
+#if LLVM_ENABLE_THREADS
+ /// Wrap the Task in a std::function<void()> that sets the result of the
+ /// corresponding future.
+ auto R = createTaskAndFuture(Task);
- // Returns the maximum number of worker threads in the pool, not the current
- // number of threads!
- unsigned getMaxConcurrency() const { return MaxThreadCount; }
+ asyncEnqueue(std::move(R.first), Group);
+ return R.second.share();
- // TODO: misleading legacy name warning!
- LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency")
- unsigned getThreadCount() const { return MaxThreadCount; }
+#else // LLVM_ENABLE_THREADS Disabled
----------------
dwblaikie wrote:
Yeah, I think that's maybe a nice place to aspire to, but I don't mind if the default implementation does the current macro conditional behaviour.
I feel a bit more strongly about the naming... Seems slightly awkward that the current implementation would keep the unadorned name and the interface would be adorned.
I think I'd be ok with this patch as-is, then a couple of renames shortly after (long enough after that this has baked) - rename the current/implementation to "DefaultThreadPool" or something and give that time to set. Then rename ThreadPoolInterface to ThreadPool?
https://github.com/llvm/llvm-project/pull/82094
More information about the llvm-commits
mailing list