[llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an impleme… (PR #82094)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 19 16:06:14 PST 2024


dwblaikie wrote:

> > I'd probably use ThreadPool as the name for the interface, and rename the current implementation to DefaultThreadPool or StdThreadPool or BasicThreadPool or the like?
> 
> That's a good point, and actually I think I considered it but it's also a much larger breaking changes: right now the refactoring would not break any user, while the renaming would require every place that instantiate a `ThreadPool` class to be updated. (I don't mind the mass rename though, it's scriptable)

Fair - I tend to be more inclined to do the mass renaming, etc. But I understand the hesitance.
 
> > It might also be an opportunity to simplify the current macro and other checking for whether multithreading is enabled - we could have an UnThreadPool that's just single threaded, and use that when threading is disabled?
> 
> That would percolate into every single caller though? Right now the `ThreadPool` class is instantiated in various places, and the implementation manages. How would you envision it if there was a `UnThreadPool` class?

Ah, sorry, didn't fully describe this - I was figuring maybe a factory function that could return the default ThreadPool implementation, making the determination to return an UnThreadPool in the case where that was the right option/when threading was disabled.

But I guess that means even more refactoring that's harder to regex - if the factory function would have to return a `std::unique_ptr<ThreadPool>` and all the access would then have to be `->` instead of `.`



https://github.com/llvm/llvm-project/pull/82094


More information about the llvm-commits mailing list