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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

LG, thanks!



================
Comment at: llvm/include/llvm/Support/ThreadPool.h:71
+  // number of threads).
+  unsigned getThreadCount() const { return MaxThreadCount; }
 
----------------
Benoit wrote:
> 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` ? 
getCurrentThreadCount() LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115019