From 0e3dc5a8be6f30a40343ae1326b853c3b2224ce9 Mon Sep 17 00:00:00 2001 From: Arnaud Bienner Date: Mon, 16 Jul 2012 00:06:55 +0200 Subject: [PATCH] Add new ConcurrentRun templates for void functions, and functions with 3 arguments + corresponding test cases. + SongLoader now has its own QThreadPool to load folders/playlist in background. Update issue 2598 This should fix slowliness problems reported. --- .../core/concurrentrun.cpp | 11 - ext/libclementine-common/core/concurrentrun.h | 209 ++++++++++++++++-- src/core/songloader.cpp | 8 +- src/core/songloader.h | 3 + src/devices/devicemanager.cpp | 2 +- src/engines/gstenginepipeline.cpp | 2 - tests/concurrentrun_test.cpp | 130 ++++++++++- 7 files changed, 331 insertions(+), 34 deletions(-) delete mode 100644 ext/libclementine-common/core/concurrentrun.cpp diff --git a/ext/libclementine-common/core/concurrentrun.cpp b/ext/libclementine-common/core/concurrentrun.cpp deleted file mode 100644 index c19f986e9..000000000 --- a/ext/libclementine-common/core/concurrentrun.cpp +++ /dev/null @@ -1,11 +0,0 @@ -#include "concurrentrun.h" - -namespace ConcurrentRun { - -void Run( - QThreadPool* threadpool, - std::tr1::function function) { - (new ThreadFunctorVoid(function))->Start(threadpool); -} - -} // namespace ConcurrentRun diff --git a/ext/libclementine-common/core/concurrentrun.h b/ext/libclementine-common/core/concurrentrun.h index 874565358..fe65dc831 100644 --- a/ext/libclementine-common/core/concurrentrun.h +++ b/ext/libclementine-common/core/concurrentrun.h @@ -20,6 +20,9 @@ #include +#include +#include + #include #include #include @@ -40,11 +43,18 @@ Run() functions are used for convenience: to directly create a new ThreadFunctor object and start it. - Currently, only functions with one or two arguments and a return value are + Currently, only functions with zero, one, two or three arguments are supported, but other might be added easily for X arguments by defining a new - ThreadFunctorBasX class, and add new Run() function. + ThreadFunctorX class (and eventually a second class for handling functions + which return void: see existing classes for exampels), and add new Run() + function. */ + +/* + Base abstract classes ThreadFunctorBase and ThreadFunctor (for void and + non-void result): +*/ template class ThreadFunctorBase : public QFutureInterface, public QRunnable { public: @@ -62,6 +72,15 @@ class ThreadFunctorBase : public QFutureInterface, public QRunnable } virtual void run() = 0; +}; + +// Base implemenation for functions having a result to be returned +template +class ThreadFunctor : public ThreadFunctorBase { + public: + ThreadFunctor() {} + + virtual void run() = 0; void End() { this->reportResult(result_); @@ -72,24 +91,64 @@ class ThreadFunctorBase : public QFutureInterface, public QRunnable ReturnType result_; }; -// Use bool as a placeholder result. -class ThreadFunctorVoid : public ThreadFunctorBase { +// Base implementation for functions with void result +template +class ThreadFunctor >::type> + : public ThreadFunctorBase { public: - ThreadFunctorVoid(std::tr1::function function) + ThreadFunctor() {} + + virtual void run() = 0; + + void End() { + this->reportFinished(); + } +}; + +/* + ThreadFunctor with no arguments: +*/ +// Non-void result +template +class ThreadFunctor0 : public ThreadFunctor { +public: + ThreadFunctor0(std::tr1::function function) + : function_(function) + { } + + void run() { + this->result_ = function_(); + ThreadFunctor::End(); + } + +private: + std::tr1::function function_; +}; + +// Void result +template +class ThreadFunctor0 >::type> + : public ThreadFunctor { +public: + ThreadFunctor0(std::tr1::function function) : function_(function) { } void run() { function_(); - ThreadFunctorBase::End(); + ThreadFunctor::End(); } - private: - std::tr1::function function_; +private: + std::tr1::function function_; }; -template -class ThreadFunctor1 : public ThreadFunctorBase { +/* + ThreadFunctor with one argument: +*/ +// Non-void result +template +class ThreadFunctor1 : public ThreadFunctor { public: ThreadFunctor1(std::tr1::function function, const Arg& arg) @@ -99,7 +158,7 @@ public: void run() { this->result_ = function_(arg_); - ThreadFunctorBase::End(); + ThreadFunctor::End(); } private: @@ -107,8 +166,33 @@ private: Arg arg_; }; -template -class ThreadFunctor2 : public ThreadFunctorBase { +// Void result +template +class ThreadFunctor1 >::type> + : public ThreadFunctor { +public: + ThreadFunctor1(std::tr1::function function, + const Arg& arg) + : function_(function), + arg_(arg) + { } + + void run() { + function_(arg_); + ThreadFunctor::End(); + } + +private: + std::tr1::function function_; + Arg arg_; +}; + +/* + ThreadFunctor with two arguments: +*/ +// Non-void result +template +class ThreadFunctor2 : public ThreadFunctor { public: ThreadFunctor2(std::tr1::function function, const Arg1& arg1, const Arg2& arg2) @@ -119,7 +203,7 @@ public: void run() { this->result_ = function_(arg1_, arg2_); - ThreadFunctorBase::End(); + ThreadFunctor::End(); } private: @@ -128,10 +212,94 @@ private: Arg2 arg2_; }; +// Void result +template +class ThreadFunctor2 >::type> + : public ThreadFunctor { +public: + ThreadFunctor2(std::tr1::function function, + const Arg1& arg1, const Arg2& arg2) + : function_(function), + arg1_(arg1), + arg2_(arg2) + { } + + void run() { + function_(arg1_, arg2_); + ThreadFunctor::End(); + } + +private: + std::tr1::function function_; + Arg1 arg1_; + Arg2 arg2_; +}; + +/* + ThreadFunctor with three arguments: +*/ +// Non-void result +template +class ThreadFunctor3 : public ThreadFunctor { +public: + ThreadFunctor3(std::tr1::function function, + const Arg1& arg1, const Arg2& arg2, const Arg3& arg3) + : function_(function), + arg1_(arg1), + arg2_(arg2), + arg3_(arg3) + { } + + void run() { + this->result_ = function_(arg1_, arg2_, arg3_); + ThreadFunctor::End(); + } + +private: + std::tr1::function function_; + Arg1 arg1_; + Arg2 arg2_; + Arg3 arg3_; +}; + +// Void result +template +class ThreadFunctor3 >::type> + : public ThreadFunctor { +public: + ThreadFunctor3(std::tr1::function function, + const Arg1& arg1, const Arg2& arg2, const Arg3& arg3) + : function_(function), + arg1_(arg1), + arg2_(arg2), + arg3_(arg3) + { } + + void run() { + function_(arg1_, arg2_, arg3_); + ThreadFunctor::End(); + } + +private: + std::tr1::function function_; + Arg1 arg1_; + Arg2 arg2_; + Arg3 arg3_; +}; + + +/* + Run functions +*/ namespace ConcurrentRun { - void Run( + + template + QFuture Run( QThreadPool* threadpool, - std::tr1::function function); + std::tr1::function function) { + + return (new ThreadFunctor0(function))->Start(threadpool); + } template QFuture Run( @@ -150,6 +318,15 @@ namespace ConcurrentRun { return (new ThreadFunctor2(function, arg1, arg2))->Start(threadpool); } + + template + QFuture Run( + QThreadPool* threadpool, + std::tr1::function function, + const Arg1& arg1, const Arg2& arg2, const Arg3& arg3) { + + return (new ThreadFunctor3(function, arg1, arg2, arg3))->Start(threadpool); + } } #endif // CONCURRENTRUN_H diff --git a/src/core/songloader.cpp b/src/core/songloader.cpp index a2f9f1ed0..f7303533b 100644 --- a/src/core/songloader.cpp +++ b/src/core/songloader.cpp @@ -17,6 +17,7 @@ #include "config.h" #include "songloader.h" +#include "core/concurrentrun.h" #include "core/logging.h" #include "core/song.h" #include "core/signalchecker.h" @@ -38,7 +39,6 @@ #include #include #include -#include #include #include @@ -229,7 +229,8 @@ SongLoader::Result SongLoader::LoadLocal(const QString& filename, bool block, // inside right away. if (QFileInfo(filename).isDir()) { if (!block) { - QtConcurrent::run(this, &SongLoader::LoadLocalDirectoryAndEmit, filename); + ConcurrentRun::Run(&thread_pool_, + boost::bind(&SongLoader::LoadLocalDirectoryAndEmit, this, filename)); return WillLoadAsync; } else { LoadLocalDirectory(filename); @@ -261,7 +262,8 @@ SongLoader::Result SongLoader::LoadLocal(const QString& filename, bool block, // It's a playlist! if (!block) { - QtConcurrent::run(this, &SongLoader::LoadPlaylistAndEmit, parser, filename); + ConcurrentRun::Run(&thread_pool_, + boost::bind(&SongLoader::LoadPlaylistAndEmit, this, parser, filename)); return WillLoadAsync; } else { LoadPlaylist(parser, filename); diff --git a/src/core/songloader.h b/src/core/songloader.h index 714a85a01..9d8fd2833 100644 --- a/src/core/songloader.h +++ b/src/core/songloader.h @@ -19,6 +19,7 @@ #define SONGLOADER_H #include +#include #include #include "song.h" @@ -129,6 +130,8 @@ private: LibraryBackendInterface* library_; boost::shared_ptr pipeline_; + + QThreadPool thread_pool_; }; #endif // SONGLOADER_H diff --git a/src/devices/devicemanager.cpp b/src/devices/devicemanager.cpp index 9796d417f..de6257a6a 100644 --- a/src/devices/devicemanager.cpp +++ b/src/devices/devicemanager.cpp @@ -187,7 +187,7 @@ DeviceManager::DeviceManager(Application* app, QObject *parent) // This reads from the database and contends on the database mutex, which can // be very slow on startup. - ConcurrentRun::Run(&thread_pool_, bind(&DeviceManager::LoadAllDevices, this)); + ConcurrentRun::Run(&thread_pool_, bind(&DeviceManager::LoadAllDevices, this)); // This proxy model only shows connected devices connected_devices_model_ = new DeviceStateFilterModel(this); diff --git a/src/engines/gstenginepipeline.cpp b/src/engines/gstenginepipeline.cpp index 41b34865c..6382eada0 100644 --- a/src/engines/gstenginepipeline.cpp +++ b/src/engines/gstenginepipeline.cpp @@ -33,8 +33,6 @@ # include "internet/spotifyservice.h" #endif -#include - const int GstEnginePipeline::kGstStateTimeoutNanosecs = 10000000; const int GstEnginePipeline::kFaderFudgeMsec = 2000; diff --git a/tests/concurrentrun_test.cpp b/tests/concurrentrun_test.cpp index f17d1511f..8010b955f 100644 --- a/tests/concurrentrun_test.cpp +++ b/tests/concurrentrun_test.cpp @@ -4,14 +4,47 @@ #include #include +#include + #include "core/concurrentrun.h" #include "test_utils.h" +int f() { + return 1337; +} + +TEST(ConcurrentRunTest, ConcurrentRun0StartAndWait) { + QThreadPool threadpool; + QFuture future = ConcurrentRun::Run(&threadpool, &f); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1337, watcher.result()); +} + +int g(int i) { + return ++i; +} + +TEST(ConcurrentRunTest, ConcurrentRun1StartAndWait) { + QThreadPool threadpool; + int i = 1336; + QFuture future = ConcurrentRun::Run(&threadpool, &g, i); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1337, watcher.result()); +} + int max(int i, int j) { return (i > j ? i : j); } -TEST(ConcurrentRunTest, ConcurrentRunStartAndWait) { +TEST(ConcurrentRunTest, ConcurrentRun2StartAndWait) { int i = 10; int j = 42; QThreadPool threadpool; @@ -24,6 +57,101 @@ TEST(ConcurrentRunTest, ConcurrentRunStartAndWait) { EXPECT_EQ(42, watcher.result()); } +int sum(int a, int b, int c) { + return a + b + c; +} + +TEST(ConcurrentRunTest, ConcurrentRun3StartAndWait) { + int i = 10; + int j = 42; + int k = 50; + QThreadPool threadpool; + QFuture future = ConcurrentRun::Run(&threadpool, &sum, i, j, k); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(102, watcher.result()); +} + +void aFunction(int* n) { + *n = 1337; +} + +void bFunction(int* n, int *m) { + aFunction(n); + *m = 1338; +} + +void cFunction(int* n, int *m, int *o) { + bFunction(n, m); + *o = 1339; +} + +TEST(ConcurrentRunTest, ConcurrentRunVoidFunction1Start) { + QThreadPool threadpool; + + int n = 10; + QFuture future = ConcurrentRun::Run(&threadpool, &aFunction, &n); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1337, n); +} + +TEST(ConcurrentRunTest, ConcurrentRunVoidFunction2Start) { + QThreadPool threadpool; + + int n = 10, m = 11; + QFuture future = ConcurrentRun::Run(&threadpool, &bFunction, &n, &m); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1337, n); + EXPECT_EQ(1338, m); +} + +TEST(ConcurrentRunTest, ConcurrentRunVoidFunction3Start) { + QThreadPool threadpool; + + int n = 10, m = 11, o = 12; + QFuture future = ConcurrentRun::Run(&threadpool, &cFunction, &n, &m, &o); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(1337, n); + EXPECT_EQ(1338, m); + EXPECT_EQ(1339, o); +} + +class A { + public: + void f(int* i) { + *i = *i + 1; + } +}; + +TEST(ConcurrentRunTest, ConcurrentRunVoidBindFunctionStart) { + QThreadPool threadpool; + + A a; + int nb = 10; + QFuture future = ConcurrentRun::Run(&threadpool, std::tr1::bind(&A::f, &a, &nb)); + QFutureWatcher watcher; + watcher.setFuture(future); + QEventLoop loop; + QObject::connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); + EXPECT_EQ(11, nb); +} + // TODO: add some more complex test cases? (e.g. with several CPU-consuming // tasks launched in parallel, with/without threadpool's threads numbers // decreased, etc.)