[PATCH] D115019: ThreadPool: grow the pool only as needed

[Benoit Jacob via Phabricator via llvm-commits]

Benoit added a comment.

In D115019#3168840 <https://reviews.llvm.org/D115019#3168840>, @mehdi_amini wrote:

> I'm fairly sure I fixed this one or two months ago, at least the C++ API allows to setup a project without starting a ThreadPool at all. This was a bottleneck in TensorFlow for some small MLIR work we're doing there.
>
> There might be some similar plumbing we could do in how we handle `--mlir-disable-threading` with `mlir-opt` as well, since it is a testing tool we haven't tried to "optimize" this kind of things (you're making a good case for it though!).

Indeed, it is in `mlir-opt` that I was observing that behavior this week.



================
Comment at: llvm/include/llvm/Support/ThreadPool.h:71
+  // number of threads).
+  unsigned getThreadCount() const { return MaxThreadCount; }
 
----------------
mehdi_amini wrote:
> I'm not fond of keeping the API name as-is with a new semantics.
> 
> What about removing it and using two APIs instead:
> 
> - getMaxThreadCount()
> - getAvailableThreadCount() => return Threads.size()
There are 3 call sites in one Clang file, and 2 call sites in 2 MLIR files. That's a little more than I feel save updating all in one shot. Are you OK to keep the current name for now, and defer to a follow-up? At least the behavior isn't changing, these existing users are still going to get effectively the same result value.

For the new methods: I agree that the current `getThreadCount` should get renamed `getMaxThreadCount`. 

I don't feel comfortable with `getAvailableThreadCount() => return Threads.size()` because that would conflict with the meaning of "available" in the existing member `AvailableThreads`.  How about `getCurrentThreadCount` ? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115019