Construct ThreadPool from vector of existing threads

LLVM Project
1

Hi all,
I was wondering whether it could be possible to use a custom set of pre-existing std::threads to run MLIR pipelines, if these threads are not already incapsulated in an llvm::ThreadPool ?

I understand that it is already possible to specify a llvm:ThreadPool in an MLIRContext, and that internally an llvm::ThreadPool stores a vector of llvm::Threads.
However it doesn’t seem to be possible to:

  • Construct an llvm::thread from an existing std::thread
  • Construct an llvm::ThreadPool from a vector of pre-existing llvm::threads or std::threads

I suppose that llvm::threads and llvm::ThreadPools were designed to be handled internally; however in some contexts the need may arise to be able to use an external thread/ThreadPool that wasn’t defined with an llvm class.
Do you think that these features could make sense?
Or maybe the “threadpool” member in the MLIRContext class could be an interface type (with the same public methods of the current llvm::ThreadPool), but with a customizable implementation?
If so, I’d be happy to propose a PR to implement them

Thank you in advance :grinning:

CC: @mehdi_amini

1 Like
2

Adding support for injecting custom thread pools implementations into the MLIRContext would be a very welcome contribution I think!
It’s something that came up from time to time, but never with an enough strong need that we got to implement it. Happy to review a patch!

3

Thank you for your answer!
On second thought, it looks like the threadpool is stored in the MLIRContext through pointers or references, thus in theory we could pass subclasses deriving from the current llvm::ThreadPool?
The public methods of llvm::ThreadPool would have to be marked as virtual, and the subclasses could override them to provide custom implementations.
This would enable custom threadpools in MLIR, with minimal modifications to the current upstream code (just adding “virtual” to the methods).
Do you think that this approach could make sense?
Thank you in advance :grinning:

1 Like
4

This is a good direction, but I would rather see the ThreadPool class have pure virtual method if we go this route, and the current TheadPool itself be a subclass implementation (Or maybe better go with a concept/model template approach instead of inheritance). Override part of the class looks like introducing too much coupling to the current implementation.

I’m not sure also if the API isn’t already leaking implementation details? (I haven’t checked just now)

1 Like
5

I just gave it a quick shot: Split the llvm::ThreadPool into an abstract base class and an impleme… by joker-eph · Pull Request #82094 · llvm/llvm-project · GitHub

6

Looks good to me, thanks for this (and sorry for not getting back to you earlier)!
My only doubt is whether it could be useful to have the ThreadPoolTaskGroup as an abstract class as well, or at least with virtual methods that could be overridden if needed?