From 6ee68583f8b2f7b5bec388d6c454b91d5a9d34a8 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Fri, 18 Oct 2013 14:19:19 +0000 Subject: SkThreadPool: allow for Runnables that add other Runnables to the pool. There's a scenario that we're currently not allowing for, but I'd really like to use in DM: 1) client calls add(SomeRunnable*) several times 2) client calls wait() 3) any of the runnables added by the client _themselves_ call add(SomeOtherRunnable*) 4-inf) maybe those SomeOtherRunnables too call add(SomeCrazyThirdRunnable*), etc. Right now in this scenario we'll assert in debug mode in step 3) when we call add() and we're waiting to stop, and do strange unspecified things in release mode. The old threadpool had basically two states: running, and waiting to stop. If a thread saw we were waiting to stop and the queue was empty, that thread shut down. This wasn't accounting for any work that other threads might be doing; potentially they were about to add to the queue. So now we have three states: running, waiting, and halting. When the client calls wait() (or the destructor triggers), we move into waiting. When a thread notices we're _really_ done, that is, have an empty queue and there are no active threads, we move into halting. The halting state actually triggers the threads to stop, which wait() is patiently join()ing on. BUG= R=bungeman@google.com, bsalomon@google.com Author: mtklein@google.com Review URL: https://codereview.chromium.org/26389005 git-svn-id: http://skia.googlecode.com/svn/trunk@11852 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/utils/SkThreadPool.cpp | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/utils/SkThreadPool.cpp b/src/utils/SkThreadPool.cpp index e078af3ba3..125a5d9b6a 100644 --- a/src/utils/SkThreadPool.cpp +++ b/src/utils/SkThreadPool.cpp @@ -28,7 +28,7 @@ static int num_cores() { } SkThreadPool::SkThreadPool(int count) -: fDone(false) { +: fState(kRunning_State), fBusyThreads(0) { if (count < 0) count = num_cores(); // Create count threads, all running SkThreadPool::Loop. for (int i = 0; i < count; i++) { @@ -39,14 +39,14 @@ SkThreadPool::SkThreadPool(int count) } SkThreadPool::~SkThreadPool() { - if (!fDone) { + if (kRunning_State == fState) { this->wait(); } } void SkThreadPool::wait() { fReady.lock(); - fDone = true; + fState = kWaiting_State; fReady.broadcast(); fReady.unlock(); @@ -55,6 +55,7 @@ void SkThreadPool::wait() { fThreads[i]->join(); SkDELETE(fThreads[i]); } + SkASSERT(fQueue.isEmpty()); } /*static*/ void SkThreadPool::Loop(void* arg) { @@ -65,8 +66,14 @@ void SkThreadPool::wait() { // We have to be holding the lock to read the queue and to call wait. pool->fReady.lock(); while(pool->fQueue.isEmpty()) { - // Is it time to die? - if (pool->fDone) { + // Does the client want to stop and are all the threads ready to stop? + // If so, we move into the halting state, and whack all the threads so they notice. + if (kWaiting_State == pool->fState && pool->fBusyThreads == 0) { + pool->fState = kHalting_State; + pool->fReady.broadcast(); + } + // Any time we find ourselves in the halting state, it's quitting time. + if (kHalting_State == pool->fState) { pool->fReady.unlock(); return; } @@ -83,14 +90,20 @@ void SkThreadPool::wait() { // Having claimed our SkRunnable, we now give up the lock while we run it. // Otherwise, we'd only ever do work on one thread at a time, which rather // defeats the point of this code. + pool->fBusyThreads++; pool->fReady.unlock(); // OK, now really do the work. r->fRunnable->run(); SkDELETE(r); + + // Let everyone know we're not busy. + pool->fReady.lock(); + pool->fBusyThreads--; + pool->fReady.unlock(); } - SkASSERT(false); // Unreachable. The only exit happens when pool->fDone. + SkASSERT(false); // Unreachable. The only exit happens when pool->fState is kHalting_State. } void SkThreadPool::add(SkRunnable* r) { @@ -105,7 +118,7 @@ void SkThreadPool::add(SkRunnable* r) { // We have some threads. Queue it up! fReady.lock(); - SkASSERT(!fDone); // We shouldn't be adding work to a pool that's shut down. + SkASSERT(fState != kHalting_State); // Shouldn't be able to add work when we're halting. LinkedRunnable* linkedRunnable = SkNEW(LinkedRunnable); linkedRunnable->fRunnable = r; fQueue.addToHead(linkedRunnable); -- cgit v1.2.3