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

[Russell Gallop via Phabricator via llvm-commits]

russell.gallop added a subscriber: MaskRay.
russell.gallop added a comment.

In D118550#3302487 <https://reviews.llvm.org/D118550#3302487>, @mehdi_amini wrote:

> (There are visible changes around the name associated with the tracer in `lld` that someone familiar with this in `lld` should approve.)

Maybe add @MaskRay.

@int3, apologies if my suggestion of adding into ThreadPool has made this more complicated! Your original change may be okay as a quicker fix, moving to ThreadPool could be a follow up.



================
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) {
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118550