[PATCH] D48240: Try again to implement a FIFO task queue

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 15 14:52:56 PDT 2018


zturner created this revision.
zturner added reviewers: labath, dblaikie, davide, inglorion.
Herald added subscribers: hiraditya, mgorny.

In https://reviews.llvm.org/D48115 I attempted to modify `ThreadPool` so that it could support tasks that returned values.  I got this working, but in doing so I added a test that relied on sequential execution of tasks that run on the same thread, and this uncovered some problems in my approach.

What I *really* wanted was to create a `ThreadPool` with a single thread, queue up asynchronous work to it, and then assume that the work would be completed sequentially.  After having some difficulty making these assumptions fit be consistent with those of the `ThreadPool` class, we decided that ultimately `ThreadPool` isn't the right place for that, and we should have a separate class explicitly for this purpose.  So that patch was reverted.

Enter `TaskQueue`.  `TaskQueue` consists of exactly 1 thread, that always executes its work first in first out.  When you queue asynchronous work to it, it returns a future that can be waited on.  When the wait returns, you are guaranteed that the work you waited on, as well as all previous work, has been completed.

Getting the `LLVM_ENABLE_THREADS=0` case working is quite a tricky job.  There may be a better solution than what I've come up with here, but this is the only way I could get it to work.  When `LLVM_ENABLE_THREADS=1`, the problem is very easy.  Just add the work to a queue, and process the queue on a single blessed background thread.  When `LLVM_ENABLE_THREADS=0` however, you have to consider the case where a user queues up multiple asynchronous tasks and then waits on them out of order.  So if you queue https://reviews.llvm.org/F1, https://reviews.llvm.org/F2, and https://reviews.llvm.org/F3, and wait on https://reviews.llvm.org/F3, https://reviews.llvm.org/F3 has to manually invoke https://reviews.llvm.org/F2's work, which has to manually invoke https://reviews.llvm.org/F1's work.  So the implementations of `LLVM_ENABLE_THREADS` and `!LLVM_ENABLE_THREADS` diverge somewhat, but I couldn't find any other way to make this work.

Note that my final goal (supporting tasks that return a value) is still unimplemented in this patch.  I plan to do that in a followup, as there are some additional complexities that need to be taken into consideration, mostly in the `LLVM_ENABLE_THREADS=0` branch.


https://reviews.llvm.org/D48240

Files:
  llvm/include/llvm/Support/TaskQueue.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TaskQueue.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/TaskQueueTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48240.151564.patch
Type: text/x-patch
Size: 10344 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180615/4b307287/attachment.bin>


More information about the llvm-commits mailing list