[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313637: Use ThreadLauncher to launch TaskPool threads 
(authored by fjricci).

Changed prior to commit:
  https://reviews.llvm.org/D37930?vs=115691=115844#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37930

Files:
  lldb/trunk/include/lldb/Host/TaskPool.h
  lldb/trunk/include/lldb/Utility/TaskPool.h
  lldb/trunk/lldb.xcodeproj/project.pbxproj
  lldb/trunk/source/Host/CMakeLists.txt
  lldb/trunk/source/Host/common/TaskPool.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Utility/CMakeLists.txt
  lldb/trunk/source/Utility/TaskPool.cpp
  lldb/trunk/unittests/Host/CMakeLists.txt
  lldb/trunk/unittests/Host/TaskPoolTest.cpp
  lldb/trunk/unittests/Utility/CMakeLists.txt
  lldb/trunk/unittests/Utility/TaskPoolTest.cpp

Index: lldb/trunk/unittests/Host/TaskPoolTest.cpp
===
--- lldb/trunk/unittests/Host/TaskPoolTest.cpp
+++ lldb/trunk/unittests/Host/TaskPoolTest.cpp
@@ -0,0 +1,43 @@
+#include "gtest/gtest.h"
+
+#include "lldb/Host/TaskPool.h"
+
+TEST(TaskPoolTest, AddTask) {
+  auto fn = [](int x) { return x * x + 1; };
+
+  auto f1 = TaskPool::AddTask(fn, 1);
+  auto f2 = TaskPool::AddTask(fn, 2);
+  auto f3 = TaskPool::AddTask(fn, 3);
+  auto f4 = TaskPool::AddTask(fn, 4);
+
+  ASSERT_EQ(10, f3.get());
+  ASSERT_EQ(2, f1.get());
+  ASSERT_EQ(17, f4.get());
+  ASSERT_EQ(5, f2.get());
+}
+
+TEST(TaskPoolTest, RunTasks) {
+  std::vector r(4);
+
+  auto fn = [](int x, int ) { y = x * x + 1; };
+
+  TaskPool::RunTasks([fn, ]() { fn(1, r[0]); }, [fn, ]() { fn(2, r[1]); },
+ [fn, ]() { fn(3, r[2]); }, [fn, ]() { fn(4, r[3]); });
+
+  ASSERT_EQ(2, r[0]);
+  ASSERT_EQ(5, r[1]);
+  ASSERT_EQ(10, r[2]);
+  ASSERT_EQ(17, r[3]);
+}
+
+TEST(TaskPoolTest, TaskMap) {
+  int data[4];
+  auto fn = [](int x) { data[x] = x * x; };
+
+  TaskMapOverInt(0, 4, fn);
+
+  ASSERT_EQ(data[0], 0);
+  ASSERT_EQ(data[1], 1);
+  ASSERT_EQ(data[2], 4);
+  ASSERT_EQ(data[3], 9);
+}
Index: lldb/trunk/unittests/Host/CMakeLists.txt
===
--- lldb/trunk/unittests/Host/CMakeLists.txt
+++ lldb/trunk/unittests/Host/CMakeLists.txt
@@ -6,6 +6,7 @@
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
+  TaskPoolTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -8,7 +8,6 @@
   StatusTest.cpp
   StringExtractorTest.cpp
   StructuredDataTest.cpp
-  TaskPoolTest.cpp
   TildeExpressionResolverTest.cpp
   TimeoutTest.cpp
   TimerTest.cpp
Index: lldb/trunk/source/Utility/CMakeLists.txt
===
--- lldb/trunk/source/Utility/CMakeLists.txt
+++ lldb/trunk/source/Utility/CMakeLists.txt
@@ -29,7 +29,6 @@
   StringLexer.cpp
   StringList.cpp
   StructuredData.cpp
-  TaskPool.cpp
   TildeExpressionResolver.cpp
   Timer.cpp
   UserID.cpp
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -53,7 +53,7 @@
 
 #include "lldb/Target/Language.h"
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
 
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
Index: lldb/trunk/source/Host/CMakeLists.txt
===
--- lldb/trunk/source/Host/CMakeLists.txt
+++ lldb/trunk/source/Host/CMakeLists.txt
@@ -31,6 +31,7 @@
   common/SoftwareBreakpoint.cpp
   common/StringConvert.cpp
   common/Symbols.cpp
+  common/TaskPool.cpp
   common/TCPSocket.cpp
   common/Terminal.cpp
   common/ThreadLauncher.cpp
Index: lldb/trunk/source/Host/common/TaskPool.cpp
===
--- lldb/trunk/source/Host/common/TaskPool.cpp
+++ lldb/trunk/source/Host/common/TaskPool.cpp
@@ -0,0 +1,109 @@
+//===- TaskPool.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Host/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
+
+#include  // for uint32_t
+#include// for queue
+#include   // for thread
+
+namespace {
+class TaskPoolImpl {
+public:
+  static TaskPoolImpl ();
+
+  void AddTask(std::function &_fn);
+
+private:
+  TaskPoolImpl();
+
+  static lldb::thread_result_t 

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.
This revision is now accepted and ready to land.

I don't really have an opinion if moving TaskPool to Host is a good or bad idea 
(haven't followed the recent layering efforts) but other then that looks good. 
I also tested it on Linux and it works fine.


https://reviews.llvm.org/D37930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

fjricci updated this revision to Diff 115691.
fjricci added a comment.
Herald added subscribers: JDevlieghere, mgorny.

Move TaskPool from Utility to Host


https://reviews.llvm.org/D37930

Files:
  include/lldb/Host/TaskPool.h
  include/lldb/Utility/TaskPool.h
  lldb.xcodeproj/project.pbxproj
  source/Host/CMakeLists.txt
  source/Host/common/TaskPool.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Utility/CMakeLists.txt
  source/Utility/TaskPool.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/TaskPoolTest.cpp
  unittests/Utility/CMakeLists.txt
  unittests/Utility/TaskPoolTest.cpp

Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -8,7 +8,6 @@
   StatusTest.cpp
   StringExtractorTest.cpp
   StructuredDataTest.cpp
-  TaskPoolTest.cpp
   TildeExpressionResolverTest.cpp
   TimeoutTest.cpp
   TimerTest.cpp
Index: unittests/Host/TaskPoolTest.cpp
===
--- unittests/Host/TaskPoolTest.cpp
+++ unittests/Host/TaskPoolTest.cpp
@@ -1,6 +1,6 @@
 #include "gtest/gtest.h"
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
 
 TEST(TaskPoolTest, AddTask) {
   auto fn = [](int x) { return x * x + 1; };
Index: unittests/Host/CMakeLists.txt
===
--- unittests/Host/CMakeLists.txt
+++ unittests/Host/CMakeLists.txt
@@ -6,6 +6,7 @@
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp
+  TaskPoolTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: source/Utility/CMakeLists.txt
===
--- source/Utility/CMakeLists.txt
+++ source/Utility/CMakeLists.txt
@@ -29,7 +29,6 @@
   StringLexer.cpp
   StringList.cpp
   StructuredData.cpp
-  TaskPool.cpp
   TildeExpressionResolver.cpp
   Timer.cpp
   UserID.cpp
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -53,7 +53,7 @@
 
 #include "lldb/Target/Language.h"
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
 
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
Index: source/Host/common/TaskPool.cpp
===
--- source/Host/common/TaskPool.cpp
+++ source/Host/common/TaskPool.cpp
@@ -7,7 +7,8 @@
 //
 //===--===//
 
-#include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, min_stack_size)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -31,6 +31,7 @@
   common/SoftwareBreakpoint.cpp
   common/StringConvert.cpp
   common/Symbols.cpp
+  common/TaskPool.cpp
   common/TCPSocket.cpp
   common/Terminal.cpp
   common/ThreadLauncher.cpp
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -2664,8 +2664,8 @@
 		6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ArmUnwindInfo.cpp; path = source/Symbol/ArmUnwindInfo.cpp; sourceTree = ""; };
 		6D9AB3DC1BB2B74E003F2289 /* TypeMap.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = TypeMap.cpp; path = source/Symbol/TypeMap.cpp; sourceTree = ""; };
 		

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

fjricci reopened this revision.
fjricci added a comment.
This revision is now accepted and ready to land.

Is ThreadLauncher unavailable in this code for some reason? The link failed on 
linux buildbots (building lldb on Darwin was fine locally): 
http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13311/steps/run%20unit%20tests/logs/stdio


Repository:
  rL LLVM

https://reviews.llvm.org/D37930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313537: Use ThreadLauncher to launch TaskPool threads 
(authored by fjricci).

Changed prior to commit:
  https://reviews.llvm.org/D37930?vs=115484=115658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37930

Files:
  lldb/trunk/source/Utility/TaskPool.cpp


Index: lldb/trunk/source/Utility/TaskPool.cpp
===
--- lldb/trunk/source/Utility/TaskPool.cpp
+++ lldb/trunk/source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, min_stack_size)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);


Index: lldb/trunk/source/Utility/TaskPool.cpp
===
--- lldb/trunk/source/Utility/TaskPool.cpp
+++ lldb/trunk/source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -45,6 +48,7 @@
 
 void TaskPoolImpl::AddTask(std::function &_fn) {
   static const uint32_t max_threads = std::thread::hardware_concurrency();
+  const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
@@ -54,10 +58,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, min_stack_size)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer accepted this revision.
tberghammer added a comment.

Looks good




Comment at: source/Utility/TaskPool.cpp:61
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();

(nit): Can you create a named constant for the min stack size as just based on 
this line I would have no idea what does that number do?


https://reviews.llvm.org/D37930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37930: Use ThreadLauncher to launch TaskPool threads

[Francis Ricci via Phabricator via lldb-commits]

fjricci created this revision.

This allows for the stack size to be configured, which isn't
possible with std::thread. Prevents overflowing the stack when
performing complex operations in the task pool on darwin,
where the default pthread stack size is only 512kb.


https://reviews.llvm.org/D37930

Files:
  source/Utility/TaskPool.cpp


Index: source/Utility/TaskPool.cpp
===
--- source/Utility/TaskPool.cpp
+++ source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -54,10 +57,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);


Index: source/Utility/TaskPool.cpp
===
--- source/Utility/TaskPool.cpp
+++ source/Utility/TaskPool.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "lldb/Utility/TaskPool.h"
+#include "lldb/Host/ThreadLauncher.h"
 
 #include  // for uint32_t
 #include// for queue
@@ -23,6 +24,8 @@
 private:
   TaskPoolImpl();
 
+  static lldb::thread_result_t WorkerPtr(void *pool);
+
   static void Worker(TaskPoolImpl *pool);
 
   std::queue> m_tasks;
@@ -54,10 +57,17 @@
 // This prevents the thread
 // from exiting prematurely and triggering a linux libc bug
 // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
-std::thread(Worker, this).detach();
+lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
+   this, nullptr, 8 * 1024 * 1024)
+.Release();
   }
 }
 
+lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
+  Worker((TaskPoolImpl *)pool);
+  return 0;
+}
+
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
 std::unique_lock lock(pool->m_tasks_mutex);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits