[PATCH] D36607: [Support/Parallel] - Do not spawn thread for single/last task.

Wed Aug 16 14:58:05 PDT 2017

ruiu added a comment.

George,

First of all, busy-waiting is a no-no, so we can't eliminate the condition variable from the thread pool. When a thread has to wait for some event to happen, it has to use a condition variable to put itself into the "blocked" state instead of busy-looping until the condition is met. That is in general true even if using busy loops makes your program faster, as your OS supports multiple processes and you don't want one process eat up all CPU resources.

This patch look fine though. But the description is a bit pointless. That is why you are getting so much questions about this patch. You generally don't want to copy-pasta code to describe what you are doing (which applies not only this patch but also your other patches). Instead, use prose to describe what you are trying to achieve with a patch. I'd update the patch description as follows:

  Do not use a task group for a very small task

  parallel_for_each_n splits a given task into small pieces of tasks and then
  passes them to background threads managed by a thread pool to process them
  in parallel. TaskGroup then waits for all tasks to be done, which is done by
  TaskGroup's destructor.

  In the previous code, all tasks were passed to background threads, and the
  main thread just waited for them to finish their jobs. This patch changes
  the logic so that the main thread processes a task just like other
  worker threads instead of just waiting for workers.

  This patch improves the performance of parallel_for_each_n for a task which
  is too small that we do not split it into multiple tasks. Previously, such task
  was submitted to another thread and the main thread waited for its completion.
  That involves multiple inter-thread synchronization which is not cheap for
  small tasks. Now, such task is processed by the main thread, so no inter-thread
  communication is necessary.

https://reviews.llvm.org/D36607