<div dir="ltr"><div><div><div>IMO the ThreadPoolExecutor from parallel.h is better than llvm::ThreadPool for two reasons:<br></div>1. Thread spawning happens in another thread (for faster startup, as indicated in the comment)<br></div>2. The work queue is a stack, not a queue, which IMO lends itself better to recursive parallelism (i.e. work that spawns parallel work, etc).  Stack == depth first == bounded growth.<br><br></div>But that transformation should be separate from this change (either llvm::ThreadPool->parallel's ThreadPoolExecutor or vice versa) and should be debated in a separate review, since there are probably going to be more opinions than actual choices ;-)<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 4, 2017 at 2:53 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I interpreted the comment about using ThreadPool as replacing the homegrown task pool in Parallel.h, which is not used on the ConcRT path.  I agree that we cannot lose the ConcRT codepath.<div><br></div><div>That said, I'm still not sure if I'm the right person to switch to make the switch over to ThreadPool.  I mean, I can try, but yea..  I'm not crazy about touching something this low-level that relates to multi-threaded code.</div></div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Thu, May 4, 2017 at 2:45 PM Michael Spencer <<a href="mailto:bigcheesegs@gmail.com" target="_blank">bigcheesegs@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div><div class="m_3093021377028695565m_-8829786433350612698gmail_signature">On Thu, May 4, 2017 at 11:41 AM, Rui Ueyama via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ruiu added a comment.<br>
<br>
If we are going to examine this file, I strongly recommend porting the same functionalities (i.e. parallel_for and parallel_for_each) on top of llvm/Support/ThreadPool.h. I don't think that is hard to do as TheadPool.h seems to provide the same functionality as this file. That would be easier than review this file again.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D32826" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32826</a><br>
<br>
<br>
<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>My issue with just using Support/ThreadPool for everything is that lld/Parallel.h uses ConcRT on Windows and is quite a bit faster than both our implementations of thread pools. I don't want to punish Windows performance. I'd also rather have our interface match whatever the standard is doing in terms of parallel task execution.</div><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra">- Michael Spencer<br></div></div></blockquote></div>
</div></div></blockquote></div><br></div>