public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] Implement a thread pool
@ 2019-10-20  3:55 Tom Tromey (Code Review)
  2019-10-20  9:55 ` Christian Biesinger (Code Review)
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-20  3:55 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

gdb/ChangeLog
2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 228 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f04d2bd..ab67332 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5e01c32..3be798a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -984,6 +984,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1485,6 +1486,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..993e097
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,119 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+thread_pool thread_pool::g_thread_pool;
+
+thread_pool::thread_pool ()
+  : m_tasks_cv (new std::condition_variable)
+{
+}
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by m_tasks_cv.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_count)
+    {
+      for (size_t i = num_threads; i < m_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv->notify_all ();
+    }
+
+  m_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv->notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      task t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv->wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..d760ad1
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,99 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t count () const
+  {
+    return m_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool ();
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* An optional is used to represent a task.  If the optional is
+     empty, then this means that the receiving thread should
+     terminate.  If the optional is non-empty, then it is an actual
+     task to evaluate.  */
+  typedef optional<std::packaged_task<void ()>> task;
+
+  /* The current thread count.  */
+  size_t m_count = 0;
+
+  /* The tasks that have not been processed yet.  */
+  std::queue<task> m_tasks;
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.
+
+     Note that this is a pointer.  The thread pool detach()s its
+     threads, so that the threads will not prevent the process from
+     exiting.  However, it was discovered that if any detached threads
+     were still waiting on a condition variable, then the condition
+     variable's destructor would wait for the threads to exit --
+     defeating the purpose.
+
+     Allocating the condition variable on the heap and simply
+     "leaking" it avoids this problem.  */
+  std::condition_variable *m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
@ 2019-10-20  9:55 ` Christian Biesinger (Code Review)
  2019-10-20 15:36 ` Tom Tromey (Code Review)
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-10-20  9:55 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches

Christian Biesinger has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h 
File gdb/gdbsupport/thread-pool.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h@80 
PS1, Line 80:   std::queue<task> m_tasks;
I would still argue that at least the queue should be heap-allocated/leaked as well, so that the background threads do not access freed memory when gdb exits and the main thread destructs the threadpool. Does this not cause an address sanitizer error for you?



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
  2019-10-20  9:55 ` Christian Biesinger (Code Review)
@ 2019-10-20 15:36 ` Tom Tromey (Code Review)
  2019-10-20 20:54 ` [review v2] " Tom Tromey (Code Review)
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-20 15:36 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h 
File gdb/gdbsupport/thread-pool.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h@80 
PS1, Line 80:   std::queue<task> m_tasks;
> I would still argue that at least the queue should be heap-allocated/leaked as well, so that the background threads do not access freed memory when gdb exits and the main thread destructs the threadpool. Does this not cause an address sanitizer error for you?

I didn't try asan.

I just didn't understand when you brought this up earlier.
I think I'll go with your original suggestion of heap-allocating
the entire thread pool and then just "leaking" the whole thing.
Sorry about that.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v2] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
  2019-10-20  9:55 ` Christian Biesinger (Code Review)
  2019-10-20 15:36 ` Tom Tromey (Code Review)
@ 2019-10-20 20:54 ` Tom Tromey (Code Review)
  2019-10-21  1:29 ` Christian Biesinger (Code Review)
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-20 20:54 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

gdb/ChangeLog
2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 223 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a57b0c7..78585e1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5e01c32..3be798a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -984,6 +984,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1485,6 +1486,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..a3ebf42
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,123 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+/* The thread pool detach()s its threads, so that the threads will not
+   prevent the process from exiting.  However, it was discovered that
+   if any detached threads were still waiting on a condition variable,
+   then the condition variable's destructor would wait for the threads
+   to exit -- defeating the purpose.
+
+   Allocating the thread pool on the heap and simply "leaking" it
+   avoids this problem.
+*/
+thread_pool *thread_pool::g_thread_pool = new thread_pool ();
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by the definition of g_thread_pool.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_count)
+    {
+      for (size_t i = num_threads; i < m_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv.notify_all ();
+    }
+
+  m_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv.notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      task t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv.wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..06b0774
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,90 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool *g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t count () const
+  {
+    return m_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool () = default;
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* An optional is used to represent a task.  If the optional is
+     empty, then this means that the receiving thread should
+     terminate.  If the optional is non-empty, then it is an actual
+     task to evaluate.  */
+  typedef optional<std::packaged_task<void ()>> task;
+
+  /* The current thread count.  */
+  size_t m_count = 0;
+
+  /* The tasks that have not been processed yet.  */
+  std::queue<task> m_tasks;
+
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.  */
+  std::condition_variable m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v2] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (2 preceding siblings ...)
  2019-10-20 20:54 ` [review v2] " Tom Tromey (Code Review)
@ 2019-10-21  1:29 ` Christian Biesinger (Code Review)
  2019-10-30 22:54 ` [review v3] " Tom Tromey (Code Review)
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-10-21  1:29 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches

Christian Biesinger has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h 
File gdb/gdbsupport/thread-pool.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172/1/gdb/gdbsupport/thread-pool.h@80 
PS1, Line 80:   std::queue<task> m_tasks;
> > I would still argue that at least the queue should be heap-allocated/leaked as well, so that the b […]
Sorry for explaining it poorly. Thanks for fixing; looks good to me.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v3] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (3 preceding siblings ...)
  2019-10-21  1:29 ` Christian Biesinger (Code Review)
@ 2019-10-30 22:54 ` Tom Tromey (Code Review)
  2019-11-22 21:40 ` Pedro Alves (Code Review)
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-30 22:54 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 223 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1cc0b83..273345b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e3433b2..b42e8d1 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -993,6 +993,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1495,6 +1496,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..a3ebf42
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,123 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+/* The thread pool detach()s its threads, so that the threads will not
+   prevent the process from exiting.  However, it was discovered that
+   if any detached threads were still waiting on a condition variable,
+   then the condition variable's destructor would wait for the threads
+   to exit -- defeating the purpose.
+
+   Allocating the thread pool on the heap and simply "leaking" it
+   avoids this problem.
+*/
+thread_pool *thread_pool::g_thread_pool = new thread_pool ();
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by the definition of g_thread_pool.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_count)
+    {
+      for (size_t i = num_threads; i < m_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv.notify_all ();
+    }
+
+  m_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv.notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      task t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv.wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..06b0774
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,90 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool *g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t count () const
+  {
+    return m_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool () = default;
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* An optional is used to represent a task.  If the optional is
+     empty, then this means that the receiving thread should
+     terminate.  If the optional is non-empty, then it is an actual
+     task to evaluate.  */
+  typedef optional<std::packaged_task<void ()>> task;
+
+  /* The current thread count.  */
+  size_t m_count = 0;
+
+  /* The tasks that have not been processed yet.  */
+  std::queue<task> m_tasks;
+
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.  */
+  std::condition_variable m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v3] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (4 preceding siblings ...)
  2019-10-30 22:54 ` [review v3] " Tom Tromey (Code Review)
@ 2019-11-22 21:40 ` Pedro Alves (Code Review)
  2019-11-22 23:50 ` [review v4] " Tom Tromey (Code Review)
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves (Code Review) @ 2019-11-22 21:40 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches

Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................


Patch Set 3: Code-Review+2

(3 comments)

LGTM.  Some cosmetic comments below.

| --- /dev/null
| +++ gdb/gdbsupport/thread-pool.h
| @@ -1,0 +45,19 @@ public:
| +
| +  ~thread_pool ();
| +  DISABLE_COPY_AND_ASSIGN (thread_pool);
| +
| +  /* Set the thread count of this thread pool.  By default, no threads
| +     are created -- the thread count must be set first.  */
| +  void set_thread_count (size_t num_threads);
| +
| +  /* Return the number of executing threads.  */
| +  size_t count () const

PS3, Line 54:

A little odd that the setter is set_thread_count, and the getter is
just count.  Shouldn't it be either "set_thread_count/thread_count" or
"set_count/count"?

| +  {
| +    return m_count;
| +  }
| +
| +  /* Post a task to the thread pool.  A future is returned, which can
| +     be used to wait for the result.  */
| +  std::future<void> post_task (std::function<void ()> func);
| +
| +private:

 ...

| @@ -1,0 +65,22 @@ private:
| +  thread_pool () = default;
| +
| +  /* The callback for each worker thread.  */
| +  void thread_function ();
| +
| +  /* An optional is used to represent a task.  If the optional is
| +     empty, then this means that the receiving thread should
| +     terminate.  If the optional is non-empty, then it is an actual
| +     task to evaluate.  */
| +  typedef optional<std::packaged_task<void ()>> task;

PS3, Line 74:

IMO this typedef obfuscates more than it helps.  I'm thinking that:

typedef std::packaged_task<void ()> task;

and then using "optional<task>" where "task" is used today would end
up being clearer.  I think it's 2 places only.

Not that much of a big deal, for sure.

| +
| +  /* The current thread count.  */
| +  size_t m_count = 0;

PS3, Line 77:

If you change the getter to thread_count, then I'd also advocate
calling this "m_thread_count".

| +
| +  /* The tasks that have not been processed yet.  */
| +  std::queue<task> m_tasks;
| +
| +  /* A condition variable and mutex that are used for communication
| +     between the main thread and the worker threads.  */
| +  std::condition_variable m_tasks_cv;
| +  std::mutex m_tasks_mutex;
| +};

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Fri, 22 Nov 2019 21:40:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v4] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (5 preceding siblings ...)
  2019-11-22 21:40 ` Pedro Alves (Code Review)
@ 2019-11-22 23:50 ` Tom Tromey (Code Review)
  2019-11-26 18:59 ` Pedro Alves (Code Review)
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-22 23:50 UTC (permalink / raw)
  To: Christian Biesinger, Pedro Alves, gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 228 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6da3318..7b004bc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-10-19  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c487b10..b07b11e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -995,6 +995,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1499,6 +1500,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..8282ea3
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,128 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+
+#if CXX_STD_THREAD
+
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+/* The thread pool detach()s its threads, so that the threads will not
+   prevent the process from exiting.  However, it was discovered that
+   if any detached threads were still waiting on a condition variable,
+   then the condition variable's destructor would wait for the threads
+   to exit -- defeating the purpose.
+
+   Allocating the thread pool on the heap and simply "leaking" it
+   avoids this problem.
+*/
+thread_pool *thread_pool::g_thread_pool = new thread_pool ();
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by the definition of g_thread_pool.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_thread_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_thread_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_thread_count)
+    {
+      for (size_t i = num_threads; i < m_thread_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv.notify_all ();
+    }
+
+  m_thread_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_thread_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv.notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      optional<task> t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv.wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
+
+#endif /* CXX_STD_THREAD */
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..e1fcb38
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,90 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool *g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t thread_count () const
+  {
+    return m_thread_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool () = default;
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* The current thread count.  */
+  size_t m_thread_count = 0;
+
+  /* A convenience typedef for the type of a task.  */
+  typedef std::packaged_task<void ()> task;
+
+  /* The tasks that have not been processed yet.  An optional is used
+     to represent a task.  If the optional is empty, then this means
+     that the receiving thread should terminate.  If the optional is
+     non-empty, then it is an actual task to evaluate.  */
+  std::queue<optional<task>> m_tasks;
+
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.  */
+  std::condition_variable m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [review v4] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (6 preceding siblings ...)
  2019-11-22 23:50 ` [review v4] " Tom Tromey (Code Review)
@ 2019-11-26 18:59 ` Pedro Alves (Code Review)
  2019-11-26 21:14 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-11-26 21:14 ` Sourceware to Gerrit sync (Code Review)
  9 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves (Code Review) @ 2019-11-26 18:59 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches

Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................


Patch Set 4: Code-Review+2


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 18:59:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (8 preceding siblings ...)
  2019-11-26 21:14 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-26 21:14 ` Sourceware to Gerrit sync (Code Review)
  9 siblings, 0 replies; 11+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-26 21:14 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, Pedro Alves, gdb-patches

The original change was created by Tom Tromey.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

gdb/ChangeLog
2019-11-26  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 228 insertions(+), 0 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 48e7ca1..7c4e2b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-11-26  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-11-26  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c487b10..b07b11e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -995,6 +995,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1499,6 +1500,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..8282ea3
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,128 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+
+#if CXX_STD_THREAD
+
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+/* The thread pool detach()s its threads, so that the threads will not
+   prevent the process from exiting.  However, it was discovered that
+   if any detached threads were still waiting on a condition variable,
+   then the condition variable's destructor would wait for the threads
+   to exit -- defeating the purpose.
+
+   Allocating the thread pool on the heap and simply "leaking" it
+   avoids this problem.
+*/
+thread_pool *thread_pool::g_thread_pool = new thread_pool ();
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by the definition of g_thread_pool.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_thread_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_thread_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_thread_count)
+    {
+      for (size_t i = num_threads; i < m_thread_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv.notify_all ();
+    }
+
+  m_thread_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_thread_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv.notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      optional<task> t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv.wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
+
+#endif /* CXX_STD_THREAD */
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..e1fcb38
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,90 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool *g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t thread_count () const
+  {
+    return m_thread_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool () = default;
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* The current thread count.  */
+  size_t m_thread_count = 0;
+
+  /* A convenience typedef for the type of a task.  */
+  typedef std::packaged_task<void ()> task;
+
+  /* The tasks that have not been processed yet.  An optional is used
+     to represent a task.  If the optional is empty, then this means
+     that the receiving thread should terminate.  If the optional is
+     non-empty, then it is an actual task to evaluate.  */
+  std::queue<optional<task>> m_tasks;
+
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.  */
+  std::condition_variable m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pushed] Implement a thread pool
  2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
                   ` (7 preceding siblings ...)
  2019-11-26 18:59 ` Pedro Alves (Code Review)
@ 2019-11-26 21:14 ` Sourceware to Gerrit sync (Code Review)
  2019-11-26 21:14 ` Sourceware to Gerrit sync (Code Review)
  9 siblings, 0 replies; 11+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-26 21:14 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches; +Cc: Pedro Alves

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/172
......................................................................

Implement a thread pool

This adds a simple thread pool to gdb.  In the end, this seemed
preferable to the approach taken in an earlier version of this series;
namely, starting threads in the parallel-foreach implementation.  This
approach reduces the overhead of starting new threads, and also lets
the user control (in a subsequent patch) exactly how many worker
threads are running.

gdb/ChangeLog
2019-11-26  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* gdbsupport/thread-pool.h: New file.
	* gdbsupport/thread-pool.c: New file.
	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
	(HFILES_NO_SRCDIR): Add thread-pool.h.

Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/thread-pool.c
A gdb/gdbsupport/thread-pool.h
4 files changed, 228 insertions(+), 0 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 48e7ca1..7c4e2b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-11-26  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
+	* gdbsupport/thread-pool.h: New file.
+	* gdbsupport/thread-pool.c: New file.
+	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
+	(HFILES_NO_SRCDIR): Add thread-pool.h.
+
 2019-11-26  Tom Tromey  <tom@tromey.com>
 
 	* event-top.h (thread_local_segv_handler): Declare.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c487b10..b07b11e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -995,6 +995,7 @@
 	gdbsupport/signals.c \
 	gdbsupport/signals-state-save-restore.c \
 	gdbsupport/tdesc.c \
+	gdbsupport/thread-pool.c \
 	gdbsupport/xml-utils.c \
 	complaints.c \
 	completer.c \
@@ -1499,6 +1500,7 @@
 	gdbsupport/signals-state-save-restore.h \
 	gdbsupport/symbol.h \
 	gdbsupport/tdesc.h \
+	gdbsupport/thread-pool.h \
 	gdbsupport/version.h \
 	gdbsupport/x86-xstate.h \
 	gdbsupport/xml-utils.h \
diff --git a/gdb/gdbsupport/thread-pool.c b/gdb/gdbsupport/thread-pool.c
new file mode 100644
index 0000000..8282ea3
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.c
@@ -0,0 +1,128 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+
+#if CXX_STD_THREAD
+
+#include "gdbsupport/thread-pool.h"
+#include "gdbsupport/alt-stack.h"
+#include "gdbsupport/block-signals.h"
+#include <algorithm>
+
+namespace gdb
+{
+
+/* The thread pool detach()s its threads, so that the threads will not
+   prevent the process from exiting.  However, it was discovered that
+   if any detached threads were still waiting on a condition variable,
+   then the condition variable's destructor would wait for the threads
+   to exit -- defeating the purpose.
+
+   Allocating the thread pool on the heap and simply "leaking" it
+   avoids this problem.
+*/
+thread_pool *thread_pool::g_thread_pool = new thread_pool ();
+
+thread_pool::~thread_pool ()
+{
+  /* Because this is a singleton, we don't need to clean up.  The
+     threads are detached so that they won't prevent process exit.
+     And, cleaning up here would be actively harmful in at least one
+     case -- see the comment by the definition of g_thread_pool.  */
+}
+
+void
+thread_pool::set_thread_count (size_t num_threads)
+{
+  std::lock_guard<std::mutex> guard (m_tasks_mutex);
+
+  /* If the new size is larger, start some new threads.  */
+  if (m_thread_count < num_threads)
+    {
+      /* Ensure that signals used by gdb are blocked in the new
+	 threads.  */
+      block_signals blocker;
+      for (size_t i = m_thread_count; i < num_threads; ++i)
+	{
+	  std::thread thread (&thread_pool::thread_function, this);
+	  thread.detach ();
+	}
+    }
+  /* If the new size is smaller, terminate some existing threads.  */
+  if (num_threads < m_thread_count)
+    {
+      for (size_t i = num_threads; i < m_thread_count; ++i)
+	m_tasks.emplace ();
+      m_tasks_cv.notify_all ();
+    }
+
+  m_thread_count = num_threads;
+}
+
+std::future<void>
+thread_pool::post_task (std::function<void ()> func)
+{
+  std::packaged_task<void ()> t (func);
+  std::future<void> f = t.get_future ();
+
+  if (m_thread_count == 0)
+    {
+      /* Just execute it now.  */
+      t ();
+    }
+  else
+    {
+      std::lock_guard<std::mutex> guard (m_tasks_mutex);
+      m_tasks.emplace (std::move (t));
+      m_tasks_cv.notify_one ();
+    }
+  return f;
+}
+
+void
+thread_pool::thread_function ()
+{
+  /* Ensure that SIGSEGV is delivered to an alternate signal
+     stack.  */
+  gdb::alternate_signal_stack signal_stack;
+
+  while (true)
+    {
+      optional<task> t;
+
+      {
+	/* We want to hold the lock while examining the task list, but
+	   not while invoking the task function.  */
+	std::unique_lock<std::mutex> guard (m_tasks_mutex);
+	while (m_tasks.empty ())
+	  m_tasks_cv.wait (guard);
+	t = std::move (m_tasks.front());
+	m_tasks.pop ();
+      }
+
+      if (!t.has_value ())
+	break;
+      (*t) ();
+    }
+}
+
+}
+
+#endif /* CXX_STD_THREAD */
diff --git a/gdb/gdbsupport/thread-pool.h b/gdb/gdbsupport/thread-pool.h
new file mode 100644
index 0000000..e1fcb38
--- /dev/null
+++ b/gdb/gdbsupport/thread-pool.h
@@ -0,0 +1,90 @@
+/* Thread pool
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_THREAD_POOL_H
+#define GDBSUPPORT_THREAD_POOL_H
+
+#include <queue>
+#include <thread>
+#include <vector>
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+#include <future>
+#include "gdbsupport/gdb_optional.h"
+
+namespace gdb
+{
+
+/* A thread pool.
+
+   There is a single global thread pool, see g_thread_pool.  Tasks can
+   be submitted to the thread pool.  They will be processed in worker
+   threads as time allows.  */
+class thread_pool
+{
+public:
+  /* The sole global thread pool.  */
+  static thread_pool *g_thread_pool;
+
+  ~thread_pool ();
+  DISABLE_COPY_AND_ASSIGN (thread_pool);
+
+  /* Set the thread count of this thread pool.  By default, no threads
+     are created -- the thread count must be set first.  */
+  void set_thread_count (size_t num_threads);
+
+  /* Return the number of executing threads.  */
+  size_t thread_count () const
+  {
+    return m_thread_count;
+  }
+
+  /* Post a task to the thread pool.  A future is returned, which can
+     be used to wait for the result.  */
+  std::future<void> post_task (std::function<void ()> func);
+
+private:
+
+  thread_pool () = default;
+
+  /* The callback for each worker thread.  */
+  void thread_function ();
+
+  /* The current thread count.  */
+  size_t m_thread_count = 0;
+
+  /* A convenience typedef for the type of a task.  */
+  typedef std::packaged_task<void ()> task;
+
+  /* The tasks that have not been processed yet.  An optional is used
+     to represent a task.  If the optional is empty, then this means
+     that the receiving thread should terminate.  If the optional is
+     non-empty, then it is an actual task to evaluate.  */
+  std::queue<optional<task>> m_tasks;
+
+  /* A condition variable and mutex that are used for communication
+     between the main thread and the worker threads.  */
+  std::condition_variable m_tasks_cv;
+  std::mutex m_tasks_mutex;
+};
+
+}
+
+#endif /* GDBSUPPORT_THREAD_POOL_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I597bb642780cb9d578ca92373d2a638efb44fe52
Gerrit-Change-Number: 172
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-11-26 21:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-20  3:55 [review] Implement a thread pool Tom Tromey (Code Review)
2019-10-20  9:55 ` Christian Biesinger (Code Review)
2019-10-20 15:36 ` Tom Tromey (Code Review)
2019-10-20 20:54 ` [review v2] " Tom Tromey (Code Review)
2019-10-21  1:29 ` Christian Biesinger (Code Review)
2019-10-30 22:54 ` [review v3] " Tom Tromey (Code Review)
2019-11-22 21:40 ` Pedro Alves (Code Review)
2019-11-22 23:50 ` [review v4] " Tom Tromey (Code Review)
2019-11-26 18:59 ` Pedro Alves (Code Review)
2019-11-26 21:14 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-26 21:14 ` Sourceware to Gerrit sync (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).