[PATCH] D118550: [Support] Have ThreadPool initialize a TimeTraceProfiler per thread

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/lib/Support/ThreadPool.cpp:37
   int newThreadCount = std::min<int>(requested, MaxThreadCount);
+  TimeTraceProfiler *MainThreadProfiler = getTimeTraceProfilerInstance();
   while (static_cast<int>(Threads.size()) < newThreadCount) {
----------------
russell.gallop wrote:
> mehdi_amini wrote:
> > It seems that we'll always use the instance initialized in the thread that calls "grow".  Also, this instance has to be setup before the call to grow, and the thread can't reinitialize it for the lifetime duration of the ThreadPool if I understand correctly.
> > 
> > I'm not sure this makes sense in the full generality of the ThreadPool API?
> > I'm not sure this makes sense in the full generality of the ThreadPool API?
> 
> Do we agree that time tracing all threads used by the ThreadPool is desirable and worth pursuing?
This could be a useful feature to have a tracing feature for the ThreadPool. I'm not sure about:
- The expected behavior in terms of threading (with respect to the thread pool creation, the growing of the pool, or the enqueuing of a task).
- The current `TimeTraceProfiler` which is non-trivially coupled to some global state, making this all harder to reason about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118550