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

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In https://reviews.llvm.org/D48240#1136052, @chandlerc wrote:

> I also wonder whether you want to decouple this serialized execution from the thread creation and management. Could you accept a thread pool instead, and enqueue work on the thread pool? You can do this by having each task on the thread pool, on completion, check the queue for more work, and if there is some, schedule a new task on the thread pool. The reason why this is interesting is because if you have, say, a system thread pool that maybe gets a lot of other work, you can intersperse these tasks on it without overloading the system. It also allows the client of this class to dictate which threads are allowable threads for these tasks to run by providing a correct thread pool. My hope is that worst case, creating a one-thread thread pool and handing it to this class would be the equivalent of what you have here, but with a more clean separation of roles. I understand it would add some complexity, for example you'd have to handle the case where no work is currently in-flight when `asyncImpl` finishes adding the task to the queue and start a new batch. But this seems reasonably tolerable.


Isn't using multiple threads strictly incompatible with the idea of it being serialized or synchronized?  With multiple threads, you will have work overlapping, which confuses the purpose of what this is for.  If someone comes up with a use case for it, then maybe, but otherwise, YAGNI?

I like your other suggestions (which I snipped from the quote), I'll try to make it work and report back if I can't.

One thing that's annoying is that you can't call `promise.set_value(V)` when `V` is `void`.  Bleh.


https://reviews.llvm.org/D48240