From: Marc Slemko Date: Thu, 20 Jul 2006 00:04:18 +0000 (+0000) Subject: Bring up of thread manager X-Git-Tag: 0.2.0~1747 X-Git-Url: https://source.supwisdom.com/gerrit/gitweb?a=commitdiff_plain;h=d466b211fdf0c357e5b29fba70099947bc493164;p=common%2Fthrift.git Bring up of thread manager facebook::thrift::concurrency::test.ThreadManagerTest::test00 Launch N tasks that block for time T, verify they all complete and that the thread manager cleans up properly when it goes out of scope git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@664725 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/lib/cpp/src/concurrency/PosixThreadFactory.cc b/lib/cpp/src/concurrency/PosixThreadFactory.cc index bac122de..3587c034 100644 --- a/lib/cpp/src/concurrency/PosixThreadFactory.cc +++ b/lib/cpp/src/concurrency/PosixThreadFactory.cc @@ -92,7 +92,7 @@ public: } } - const Runnable* runnable() const {return _runnable;} + Runnable* runnable() const {return _runnable;} }; diff --git a/lib/cpp/src/concurrency/Thread.h b/lib/cpp/src/concurrency/Thread.h index 3fc094d5..8237091a 100644 --- a/lib/cpp/src/concurrency/Thread.h +++ b/lib/cpp/src/concurrency/Thread.h @@ -43,7 +43,7 @@ class Thread { /** Gets the runnable object this thread is hosting */ - virtual const Runnable* runnable() const = 0; + virtual Runnable* runnable() const = 0; }; /** Factory to create platform-specific thread object and bind them to Runnable object for execution */ diff --git a/lib/cpp/src/concurrency/ThreadManager.cc b/lib/cpp/src/concurrency/ThreadManager.cc index d13ce7ba..f8a5c224 100644 --- a/lib/cpp/src/concurrency/ThreadManager.cc +++ b/lib/cpp/src/concurrency/ThreadManager.cc @@ -1,4 +1,5 @@ #include "ThreadManager.h" +#include "Exception.h" #include "Monitor.h" #include @@ -10,12 +11,7 @@ namespace facebook { namespace thrift { namespace concurrency { /** ThreadManager class This class manages a pool of threads. It uses a ThreadFactory to create threads. It never actually creates or destroys worker threads, rather - it maintains statistics on number of idle threads, number of active threads, task backlog, and average wait and service times and informs the - PoolPolicy object bound to instances of this manager of interesting transitions. It is then up the PoolPolicy object to decide if the thread pool - size needs to be adjusted and call this object addThread and removeThread methods to make changes. - - This design allows different policy implementations to used this code to handle basic worker thread management and worker task execution and focus on - policy issues. The simplest policy, StaticPolicy, does nothing other than create a fixed number of threads. + it maintains statistics on number of idle threads, number of active threads, task backlog, and average wait and service times. @author marc @version $Id:$ */ @@ -24,34 +20,18 @@ class ThreadManager::Impl : public ThreadManager { public: - Impl(size_t highWatermark, size_t lowWatermark) : - _hiwat(highWatermark), - _lowat(lowWatermark) { - } - - ~Impl() {} - - size_t highWatermark() const {return _hiwat;} - - void highWatermark(size_t value) {_hiwat = value;} + Impl() : _stopped(false) {} - size_t lowWatermark() const {return _lowat;} - void lowWatermark(size_t value) {_lowat = value;} - const PoolPolicy* poolPolicy() const { + ~Impl() { - Synchronized s(_monitor); - - return _poolPolicy; + if(!_stopped) { + stop(); + } } - - void poolPolicy(const PoolPolicy* value) { - Synchronized s(_monitor); - - _poolPolicy = value; - } + void stop(); const ThreadFactory* threadFactory() const { @@ -67,9 +47,9 @@ class ThreadManager::Impl : public ThreadManager { _threadFactory = value; } - void addThread(size_t value); + void addWorker(size_t value); - void removeThread(size_t value); + void removeWorker(size_t value); size_t idleWorkerCount() const {return _idleCount;} @@ -77,7 +57,7 @@ class ThreadManager::Impl : public ThreadManager { Synchronized s(_monitor); - return _workers.size(); + return _workerCount; } size_t pendingTaskCount() const { @@ -91,7 +71,7 @@ class ThreadManager::Impl : public ThreadManager { Synchronized s(_monitor); - return _tasks.size() + _workers.size() - _idleCount; + return _tasks.size() + _workerCount - _idleCount; } void add(Runnable* value); @@ -100,13 +80,11 @@ class ThreadManager::Impl : public ThreadManager { private: - size_t _hiwat; - - size_t _lowat; + size_t _workerCount; size_t _idleCount; - const PoolPolicy* _poolPolicy;; + bool _stopped; const ThreadFactory* _threadFactory; @@ -148,6 +126,8 @@ public: private: Runnable* _runnable; + + friend class ThreadManager::Worker; STATE _state; }; @@ -181,6 +161,10 @@ class ThreadManager::Worker: public Runnable { if(_state == STARTING) { _state = STARTED; } + + _manager->_workerCount++; + + _manager->_monitor.notify(); } do { @@ -207,22 +191,43 @@ class ThreadManager::Worker: public Runnable { } if(_state == STARTED) { + + if(!_manager->_tasks.empty()) { - task = _manager->_tasks.front(); + task = _manager->_tasks.front(); + + _manager->_tasks.pop(); + + if(task->_state == ThreadManager::Task::WAITING) { + + task->_state = ThreadManager::Task::EXECUTING; + } + } } } if(task != NULL) { - task->run(); + if(task->_state == ThreadManager::Task::EXECUTING) { + try { + + task->run(); - delete task; + } catch(...) { + + // XXX need to log this + } + + delete task; + } } - + } while(_state == STARTED); {Synchronized s(_manager->_monitor); + _manager->_workerCount--; + if(_state == STOPPING) { _state = STOPPED; @@ -246,35 +251,56 @@ class ThreadManager::Worker: public Runnable { bool _idle; }; -void ThreadManager::Impl::addThread(size_t value) { +void ThreadManager::Impl::addWorker(size_t value) { - std::set newThreads; + std::set newThreads; - for(size_t ix = 0; ix < value; ix++) { + for(size_t ix = 0; ix < value; ix++) { - class ThreadManager::Worker; + class ThreadManager::Worker; - ThreadManager::Worker* worker = new ThreadManager::Worker(this); + ThreadManager::Worker* worker = new ThreadManager::Worker(this); - newThreads.insert(_threadFactory->newThread(worker)); - } + newThreads.insert(_threadFactory->newThread(worker)); + } - for(std::set::iterator ix = newThreads.begin(); ix != newThreads.end(); ix++) { + for(std::set::iterator ix = newThreads.begin(); ix != newThreads.end(); ix++) { - (*ix)->start(); - } - for(std::set::iterator ix = newThreads.begin(); ix != newThreads.end(); ix++) { + ThreadManager::Worker* worker = (ThreadManager::Worker*)(*ix)->runnable(); - (*ix)->start(); - } - - {Synchronized s(_monitor); + worker->_state = ThreadManager::Worker::STARTING; + + (*ix)->start(); + } + + {Synchronized s(_monitor); - _workers.insert(newThreads.begin(), newThreads.end()); + _workers.insert(newThreads.begin(), newThreads.end()); + + while(_workerCount != _workers.size()) { + _monitor.wait(); } } +} + +void ThreadManager::Impl::stop() { + + bool doStop = false; + + {Synchronized s(_monitor); + + if(!_stopped) { + doStop = true; + _stopped = true; + } + } + + if(doStop) { + removeWorker(_workerCount); + } +} -void ThreadManager::Impl::removeThread(size_t value) { +void ThreadManager::Impl::removeWorker(size_t value) { std::set removedThreads; @@ -285,14 +311,19 @@ void ThreadManager::Impl::removeThread(size_t value) { First time through, (idleOnly == 1) just look for idle threads. If that didn't find enough, go through again (idleOnly == 0) and remove a sufficient number of busy threads. */ - for(int idleOnly = 1; idleOnly <= 0; idleOnly--) { + for(int idleOnly = 1; idleOnly >= 0; idleOnly--) { for(std::set::iterator workerThread = _workers.begin(); (workerThread != _workers.end()) && (removedThreads.size() < value); workerThread++) { Worker* worker = (Worker*)(*workerThread)->runnable(); if(worker->_idle || !idleOnly) { - + + if(worker->_state == ThreadManager::Worker::STARTED) { + + worker->_state = ThreadManager::Worker::STOPPING; + } + removedThreads.insert(*workerThread); _workers.erase(workerThread); @@ -320,15 +351,17 @@ void ThreadManager::Impl::add(Runnable* value) { Synchronized s(_monitor); + bool isEmpty = _tasks.empty(); + _tasks.push(new ThreadManager::Task(value)); - /* If queue is empty notify a thread, otherwise all worker threads are running and will get around to this + /* If queue was empty notify a thread, otherwise all worker threads are running and will get around to this task in time. */ - if(_tasks.size() == 1) { - - assert(_idleCount == _workers.size()); + if(isEmpty) { + assert(_idleCount == _workerCount); + _monitor.notify(); } } @@ -336,39 +369,54 @@ void ThreadManager::Impl::add(Runnable* value) { void ThreadManager::Impl::remove(Runnable* task) { Synchronized s(_monitor); - } - -ThreadManager* ThreadManager::newThreadManager(size_t lowWatermark, size_t highWatermark) { - return new ThreadManager::Impl(lowWatermark, highWatermark); } -/** Basic Pool Policy Implementation */ +class SimpleThreadManager : public ThreadManager::Impl { -class BasicPoolPolicy::Impl : public PoolPolicy { +public: - public: + SimpleThreadManager(size_t workerCount=4) : + _workerCount(workerCount), + _firstTime(true) { + } - Impl() {} + void add(Runnable* task) { - ~Impl() {} + bool addWorkers = false; - void onEmpty(ThreadManager* source) const {} + {Synchronized s(_monitor); - void onLowWatermark(ThreadManager* source) const {} + if(_firstTime) { - void onHighWatermark(ThreadManager* source) const {} -}; + _firstTime = false; + + addWorkers = true; + } + } -BasicPoolPolicy::BasicPoolPolicy() : _impl(new BasicPoolPolicy::Impl()) {} + if(addWorkers) { -BasicPoolPolicy::~BasicPoolPolicy() { delete _impl;} + addWorker(_workerCount); + } -void BasicPoolPolicy::onEmpty(ThreadManager* source) const {_impl->onEmpty(source);} + Impl::add(task); + } -void BasicPoolPolicy::onLowWatermark(ThreadManager* source) const {_impl->onLowWatermark(source);} +private: -void BasicPoolPolicy::onHighWatermark(ThreadManager* source) const {_impl->onHighWatermark(source);} + const size_t _workerCount; + bool _firstTime; + Monitor _monitor; +}; +ThreadManager* ThreadManager::newThreadManager() { + return new ThreadManager::Impl(); +} + +ThreadManager* ThreadManager::newSimpleThreadManager() { + return new SimpleThreadManager(); +} + }}} // facebook::thrift::concurrency diff --git a/lib/cpp/src/concurrency/ThreadManager.h b/lib/cpp/src/concurrency/ThreadManager.h index 352afb9b..3b87c214 100644 --- a/lib/cpp/src/concurrency/ThreadManager.h +++ b/lib/cpp/src/concurrency/ThreadManager.h @@ -14,53 +14,12 @@ namespace facebook { namespace thrift { namespace concurrency { class ThreadManager; -/** PoolPolicy class - - Tracks performance of ThreadManager object and makes desired changes in thread pool count if any. */ - -class PoolPolicy { - - public: - - PoolPolicy() {} - - virtual ~PoolPolicy() {} - - virtual void onEmpty(ThreadManager* source) const = 0; - - virtual void onLowWatermark(ThreadManager* source) const = 0; - - virtual void onHighWatermark(ThreadManager* source) const = 0; - -}; - -class BasicPoolPolicy : public PoolPolicy { - - public: - - BasicPoolPolicy(); - - virtual ~BasicPoolPolicy(); - - virtual void onEmpty(ThreadManager* source) const; - - virtual void onLowWatermark(ThreadManager* source) const; - - virtual void onHighWatermark(ThreadManager* source) const; - - private: - - class Impl; - - Impl* _impl; -}; - /** ThreadManager class This class manages a pool of threads. It uses a ThreadFactory to create threads. It never actually creates or destroys worker threads, rather it maintains statistics on number of idle threads, number of active threads, task backlog, and average wait and service times and informs the PoolPolicy object bound to instances of this manager of interesting transitions. It is then up the PoolPolicy object to decide if the thread pool - size needs to be adjusted and call this object addThread and removeThread methods to make changes. + size needs to be adjusted and call this object addWorker and removeWorker methods to make changes. This design allows different policy implementations to used this code to handle basic worker thread management and worker task execution and focus on policy issues. The simplest policy, StaticPolicy, does nothing other than create a fixed number of threads. */ @@ -69,29 +28,17 @@ class ThreadManager { public: - ThreadManager(size_t highWatermark=4, size_t lowWatermark=2) {}; - - virtual ~ThreadManager() {}; + ThreadManager() {} - virtual const PoolPolicy* poolPolicy() const = 0; - - virtual void poolPolicy(const PoolPolicy* value) = 0; + virtual ~ThreadManager() {} virtual const ThreadFactory* threadFactory() const = 0; virtual void threadFactory(const ThreadFactory* value) = 0; - virtual size_t highWatermark() const = 0; - - virtual void highWatermark(size_t value) = 0; + virtual void addWorker(size_t value=1) = 0; - virtual size_t lowWatermark() const = 0; - - virtual void lowWatermark(size_t value) = 0; - - virtual void addThread(size_t value=1) = 0; - - virtual void removeThread(size_t value=1) = 0; + virtual void removeWorker(size_t value=1) = 0; /** Gets the current number of idle worker threads */ @@ -117,7 +64,9 @@ class ThreadManager { virtual void remove(Runnable* task) = 0; - static ThreadManager* newThreadManager(size_t lowWatermark=2, size_t highWatermark=4); + static ThreadManager* newThreadManager(); + + static ThreadManager* newSimpleThreadManager(); class Task; diff --git a/lib/cpp/src/concurrency/TimerManager.cc b/lib/cpp/src/concurrency/TimerManager.cc index 93b0dc54..7952122c 100644 --- a/lib/cpp/src/concurrency/TimerManager.cc +++ b/lib/cpp/src/concurrency/TimerManager.cc @@ -251,7 +251,6 @@ void TimerManager::stop() { delete _dispatcher; } - } const ThreadFactory* TimerManager::threadFactory() const { diff --git a/lib/cpp/src/concurrency/TimerManager.h b/lib/cpp/src/concurrency/TimerManager.h index 2681a5af..2b92f232 100644 --- a/lib/cpp/src/concurrency/TimerManager.h +++ b/lib/cpp/src/concurrency/TimerManager.h @@ -66,11 +66,11 @@ class TimerManager { virtual void remove(Runnable* task); enum STATE { - UNINITIALIZED = 1000, - STARTING = 1001, - STARTED = 1002, - STOPPING = 1003, - STOPPED = 1004 + UNINITIALIZED, + STARTING, + STARTED, + STOPPING, + STOPPED }; virtual const STATE state() const; diff --git a/lib/cpp/src/concurrency/test/Tests.cc b/lib/cpp/src/concurrency/test/Tests.cc index 36f29ff0..02e1724d 100644 --- a/lib/cpp/src/concurrency/test/Tests.cc +++ b/lib/cpp/src/concurrency/test/Tests.cc @@ -3,6 +3,7 @@ #include "ThreadFactoryTests.h" #include "TimerManagerTests.h" +#include "ThreadManagerTests.h" int main(int argc, char** argv) { @@ -48,5 +49,16 @@ int main(int argc, char** argv) { assert(timerManagerTests.test00()); } + + if(runAll || arg.compare("thread-manager") == 0) { + + std::cout << "ThreadManager tests..." << std::endl; + + std::cout << "\t\tThreadManager test00" << std::endl; + + ThreadManagerTests threadManagerTests; + + assert(threadManagerTests.test00()); + } } diff --git a/lib/cpp/src/concurrency/test/TimerManagerTests.h b/lib/cpp/src/concurrency/test/TimerManagerTests.h index f34f8b0b..3c7fc0bc 100644 --- a/lib/cpp/src/concurrency/test/TimerManagerTests.h +++ b/lib/cpp/src/concurrency/test/TimerManagerTests.h @@ -119,5 +119,3 @@ public: }}}} // facebook::thrift::concurrency -using namespace facebook::thrift::concurrency::test; -