fix allocating thread pool workers

Tasks are removed from the queue before a worker starts working
on it, which means that maTasks.size() is not the number of tasks
to do, because the worked on tasks are not included there.
This means the code could spawn only a smaller number of workers
than were needed (and than CPU cores that are available).

Change-Id: Ic6e6a79316cf48d82f2b80be7ad477b723b2c4e5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103955
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

1 files changed, 22 insertions, 2 deletions

diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx
index f93400d96f9f..906189202cdd 100644
--- a/comphelper/source/misc/threadpool.cxx
+++ b/comphelper/source/misc/threadpool.cxx
@@ -79,12 +79,14 @@ public:
             std::unique_ptr<ThreadTask> pTask = mpPool->popWorkLocked( aGuard, true );
             if( pTask )
             {
+                mpPool->incBusyWorker();
                 aGuard.unlock();
 
                 pTask->exec();
                 pTask.reset();
 
                 aGuard.lock();
+                mpPool->decBusyWorker();
             }
         }
     }
@@ -92,7 +94,8 @@ public:
 
 ThreadPool::ThreadPool(sal_Int32 nWorkers)
     : mbTerminate(true)
-    , mnWorkers(nWorkers)
+    , mnMaxWorkers(nWorkers)
+    , mnBusyWorkers(0)
 {
 }
 
@@ -104,6 +107,7 @@ ThreadPool::~ThreadPool()
     // still 0, but hopefully they will be more helpful on non-WNT platforms
     assert(mbTerminate);
     assert(maTasks.empty());
+    assert(mnBusyWorkers == 0);
 }
 
 namespace {
@@ -198,7 +202,8 @@ void ThreadPool::pushTask( std::unique_ptr<ThreadTask> pTask )
 
     mbTerminate = false;
 
-    if (maWorkers.size() < mnWorkers && maWorkers.size() <= maTasks.size())
+    // Worked on tasks are already removed from maTasks, so include the count of busy workers.
+    if (maWorkers.size() < mnMaxWorkers && maWorkers.size() <= maTasks.size() + mnBusyWorkers)
     {
         maWorkers.push_back( new ThreadWorker( this ) );
         maWorkers.back()->launch();
@@ -230,6 +235,17 @@ std::unique_ptr<ThreadTask> ThreadPool::popWorkLocked( std::unique_lock< std::mu
     return nullptr;
 }
 
+void ThreadPool::incBusyWorker()
+{
+    ++mnBusyWorkers;
+}
+
+void ThreadPool::decBusyWorker()
+{
+    --mnBusyWorkers;
+    assert(mnBusyWorkers >= 0);
+}
+
 void ThreadPool::waitUntilDone(const std::shared_ptr<ThreadTaskTag>& rTag, bool bJoinAll)
 {
 #if defined DBG_UTIL && (defined LINUX || defined _WIN32)
@@ -294,6 +310,10 @@ void ThreadTask::exec()
     {
         SAL_WARN("comphelper", "exception in thread worker while calling doWork(): " << e);
     }
+    catch (...)
+    {
+        SAL_WARN("comphelper", "unknown exception in thread worker while calling doWork()");
+    }
 
     pTag->onTaskWorkerDone();
 }