[Lldb-commits] [PATCH] D13727: Add task pool to LLDB

[Pavel Labath via lldb-commits]

labath added a comment.

Zachary, I don't think using std::async is a good idea because it provides a very different threading model than the one we want here. Let me demonstrate that with an example:

  #include <condition_variable>
  #include <future>
  #include <mutex>
  #include <thread>
  #include <vector>
  
  using namespace std;
  
  
  unsigned X = 0;
  mutex m;
  condition_variable cv;
  
  int f()
  {
      unique_lock<mutex> l(m);
      X++;
      printf("X = %d\n", X);
      cv.wait(l, [] { return X > 1000; });
      cv.notify_one();
      return X;
  }
  
  int main()
  {
      vector<future<int>> v;
      for(unsigned i = 0; i < 1000; ++i) 
          v.push_back(async(launch::async, f));
  
      v.push_back(async(launch::async, f)); // break here
  
      for(auto &f: v)
          printf("future = %d\n", f.get());
  
      return 0;
  }

This (convoluted) example starts 1000 (+1) asynchronous tasks and wires them up in a way that they can't complete until all of them are running. If the implementation was limiting the number of concurrently running tasks to n<1000, then this program would not complete. Nevertheless, it *does* complete.

When I run this program under linux, it completes instantly. That's because linux implementation of std::async just kicks off a new thread for each task let's them run freely. Windows (for better or for worse) does something different here. Initially, it only spawns a small number of tasks and then periodically checks if the tasks have finished, and if not, it starts a more of them. That's why it takes this program several minutes to complete, but it still completes, and if you check it with a debugger in the end, you will see that there were 1000 threads running (i.e. it was not doing anything clever, like multiplexing multiple tasks over the same OS thread, etc.). This is not the threading model we want here I think (in fact we should probably ban using any communication between the tasks in the pool). Our requirement seems to be "being able to limit the number of tasks running concurrently". Having the thread pool exhibit one behavior on windows and another elsewhere would be very confusing and error-prone.

Having said that, I do think there is one more thing that speaks against having hardware_concurrency() threads running constantly, that we haven't considered yet. It is our test suite. It already spawns a large number of LLDB processes in parallel, and having each process spawn a large number of threads might be dangerous. Tamas, if you're going to keep the threads alive, I think we should evaluate the impact of it on our test suite.

Considering all of this, I think it is a good idea to shut down threads when they are not used as an initial implementation. Later, we can evaluate potential improvements, like keeping some number of threads on stand-by.


http://reviews.llvm.org/D13727