From ab5ccf69da293fdfcd32c0ff11e4a6f38e2589e1 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Sun, 26 Feb 2012 15:05:46 +0000 Subject: [PATCH] Refactoring: remove BackgroundThread --- src/CMakeLists.txt | 2 - src/core/backgroundthread.cpp | 64 ------- src/core/backgroundthread.h | 185 ------------------- src/core/utilities.cpp | 27 +++ src/core/utilities.h | 17 ++ src/covers/albumcoverloader.h | 1 - src/devices/filesystemdevice.cpp | 39 ++-- src/devices/filesystemdevice.h | 4 +- src/globalsearch/globalsearch.cpp | 1 + src/globalsearch/groovesharksearchprovider.h | 1 - src/internet/internetmodel.h | 1 - src/internet/spotifyblobdownloader.cpp | 1 + src/library/library.cpp | 82 ++++---- src/library/library.h | 12 +- src/library/librarywatcher.cpp | 3 + 15 files changed, 109 insertions(+), 331 deletions(-) delete mode 100644 src/core/backgroundthread.cpp delete mode 100644 src/core/backgroundthread.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 533295f30..7bd126307 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -74,7 +74,6 @@ set(SOURCES core/appearance.cpp core/application.cpp core/backgroundstreams.cpp - core/backgroundthread.cpp core/commandlineoptions.cpp core/crashreporting.cpp core/database.cpp @@ -344,7 +343,6 @@ set(HEADERS core/application.h core/backgroundstreams.h - core/backgroundthread.h core/crashreporting.h core/database.h core/deletefiles.h diff --git a/src/core/backgroundthread.cpp b/src/core/backgroundthread.cpp deleted file mode 100644 index 3736e7754..000000000 --- a/src/core/backgroundthread.cpp +++ /dev/null @@ -1,64 +0,0 @@ -/* This file is part of Clementine. - Copyright 2010, David Sansome - - Clementine is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - Clementine is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Clementine. If not, see . -*/ - -#include "backgroundthread.h" - -BackgroundThreadBase::BackgroundThreadBase(QObject *parent) - : QThread(parent), - io_priority_(IOPRIO_CLASS_NONE), - cpu_priority_(InheritPriority) -{ -} - -int BackgroundThreadBase::SetIOPriority() { -#ifdef Q_OS_LINUX - return syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, gettid(), - 4 | io_priority_ << IOPRIO_CLASS_SHIFT); -#elif defined(Q_OS_DARWIN) - return setpriority(PRIO_DARWIN_THREAD, 0, - io_priority_ == IOPRIO_CLASS_IDLE ? PRIO_DARWIN_BG : 0); -#else - return 0; -#endif -} - -int BackgroundThreadBase::gettid() { -#ifdef Q_OS_LINUX - return syscall(SYS_gettid); -#else - return 0; -#endif -} - -void BackgroundThreadBase::Start(bool block) { - if (!block) { - // Just start the thread and return immediately - start(cpu_priority_); - return; - } - - // Lock the mutex so the new thread won't try to wake us up before we start - // waiting. - QMutexLocker l(&started_wait_condition_mutex_); - - // Start the thread. - start(cpu_priority_); - - // Wait for the thread to initalise. - started_wait_condition_.wait(l.mutex()); -} - diff --git a/src/core/backgroundthread.h b/src/core/backgroundthread.h deleted file mode 100644 index bccf4495d..000000000 --- a/src/core/backgroundthread.h +++ /dev/null @@ -1,185 +0,0 @@ -/* This file is part of Clementine. - Copyright 2010, David Sansome - - Clementine is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - Clementine is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Clementine. If not, see . -*/ - -#ifndef BACKGROUNDTHREAD_H -#define BACKGROUNDTHREAD_H - -#include -#include -#include -#include -#include - -#include - -#ifdef Q_OS_LINUX -# include -#endif -#ifdef Q_OS_DARWIN -# include -#endif - -// These classes are a bit confusing because they're trying to do so many -// things: -// * Run a worker in a background thread -// * ... or maybe run it in the same thread if we're in a test -// * Use interfaces throughout, so the implementations can be mocked -// * Create concrete implementations of the interfaces when threads start -// -// The types you should use throughout your header files are: -// BackgroundThread -// BackgroundThreadFactory -// -// You should allow callers to set their own factory (which might return mocks -// of your interface), and default to using a: -// BackgroundThreadFactoryImplementation - - -// This is the base class. We need one because moc doesn't like templated -// classes. This also deals with anything that doesn't depend on the type of -// the worker. -class BackgroundThreadBase : public QThread { - Q_OBJECT - public: - BackgroundThreadBase(QObject* parent = 0); - - // Borrowed from schedutils - enum IoPriority { - IOPRIO_CLASS_NONE = 0, - IOPRIO_CLASS_RT, - IOPRIO_CLASS_BE, - IOPRIO_CLASS_IDLE, - }; - - void set_io_priority(IoPriority priority) { io_priority_ = priority; } - void set_cpu_priority(QThread::Priority priority) { cpu_priority_ = priority; } - - virtual void Start(bool block = false); - - signals: - void Initialised(); - - protected: - int SetIOPriority(); - static int gettid(); - - enum { - IOPRIO_WHO_PROCESS = 1, - IOPRIO_WHO_PGRP, - IOPRIO_WHO_USER, - }; - static const int IOPRIO_CLASS_SHIFT = 13; - - IoPriority io_priority_; - QThread::Priority cpu_priority_; - - QWaitCondition started_wait_condition_; - QMutex started_wait_condition_mutex_; -}; - -// This is the templated class that stores and returns the worker object. -template -class BackgroundThread : public BackgroundThreadBase { - public: - BackgroundThread(QObject* parent = 0); - ~BackgroundThread(); - - boost::shared_ptr Worker() const { return worker_; } - - protected: - boost::shared_ptr worker_; -}; - -// This class actually creates an implementation of the worker object -template -class BackgroundThreadImplementation : public BackgroundThread { - public: - BackgroundThreadImplementation(QObject* parent = 0); - - protected: - void run(); -}; - - -// This is a pure virtual factory for creating threads. -template -class BackgroundThreadFactory { - public: - virtual ~BackgroundThreadFactory() {} - virtual BackgroundThread* GetThread(QObject* parent) = 0; -}; - -// This implementation of the factory returns a BackgroundThread that creates -// the right derived types... -template -class BackgroundThreadFactoryImplementation : public BackgroundThreadFactory { - public: - BackgroundThread* GetThread(QObject* parent) { - return new BackgroundThreadImplementation(parent); - } -}; - -template -BackgroundThread::BackgroundThread(QObject *parent) - : BackgroundThreadBase(parent) -{ -} - -template -BackgroundThread::~BackgroundThread() { - if (isRunning()) { - if (boost::shared_ptr w = worker_) - w->Stop(); - - quit(); - if (wait(1000)) - return; - terminate(); - wait(1000); - } -} - -template -BackgroundThreadImplementation:: - BackgroundThreadImplementation(QObject* parent) - : BackgroundThread(parent) -{ -} - - -template -void BackgroundThreadImplementation::run() { - this->SetIOPriority(); - - this->worker_.reset(new DerivedType); - - { - // Tell the calling thread that we've initialised the worker. - QMutexLocker l(&this->started_wait_condition_mutex_); - this->started_wait_condition_.wakeAll(); - } - - emit this->Initialised(); - QThread::exec(); - - this->worker_.reset(); -} - - - - -#endif // BACKGROUNDTHREAD_H diff --git a/src/core/utilities.cpp b/src/core/utilities.cpp index 6996d7cef..c31091f15 100644 --- a/src/core/utilities.cpp +++ b/src/core/utilities.cpp @@ -43,6 +43,13 @@ # include #endif +#ifdef Q_OS_LINUX +# include +#endif +#ifdef Q_OS_DARWIN +# include +#endif + #ifdef Q_OS_DARWIN # include "core/mac_startup.h" # include "CoreServices/CoreServices.h" @@ -407,6 +414,26 @@ const char* EnumToString(const QMetaObject& meta, const char* name, int value) { return result; } +int SetThreadIOPriority(IoPriority priority) { +#ifdef Q_OS_LINUX + return syscall(SYS_ioprio_set, IOPRIO_WHO_PROCESS, GetThreadId(), + 4 | priority << IOPRIO_CLASS_SHIFT); +#elif defined(Q_OS_DARWIN) + return setpriority(PRIO_DARWIN_THREAD, 0, + priority == IOPRIO_CLASS_IDLE ? PRIO_DARWIN_BG : 0); +#else + return 0; +#endif +} + +int GetThreadId() { +#ifdef Q_OS_LINUX + return syscall(SYS_gettid); +#else + return 0; +#endif +} + } // namespace Utilities diff --git a/src/core/utilities.h b/src/core/utilities.h index 0d24c9673..837ff4c7a 100644 --- a/src/core/utilities.h +++ b/src/core/utilities.h @@ -100,6 +100,23 @@ namespace Utilities { // Returns the minor version of OS X (ie. 6 for Snow Leopard, 7 for Lion). qint32 GetMacVersion(); + + // Borrowed from schedutils + enum IoPriority { + IOPRIO_CLASS_NONE = 0, + IOPRIO_CLASS_RT, + IOPRIO_CLASS_BE, + IOPRIO_CLASS_IDLE, + }; + enum { + IOPRIO_WHO_PROCESS = 1, + IOPRIO_WHO_PGRP, + IOPRIO_WHO_USER, + }; + static const int IOPRIO_CLASS_SHIFT = 13; + + int SetThreadIOPriority(IoPriority priority); + int GetThreadId(); } class ScopedWCharArray { diff --git a/src/covers/albumcoverloader.h b/src/covers/albumcoverloader.h index 442716b33..87ad717fd 100644 --- a/src/covers/albumcoverloader.h +++ b/src/covers/albumcoverloader.h @@ -19,7 +19,6 @@ #define ALBUMCOVERLOADER_H #include "albumcoverloaderoptions.h" -#include "core/backgroundthread.h" #include "core/song.h" #include diff --git a/src/devices/filesystemdevice.cpp b/src/devices/filesystemdevice.cpp index 7308bf315..8575e5fef 100644 --- a/src/devices/filesystemdevice.cpp +++ b/src/devices/filesystemdevice.cpp @@ -23,6 +23,7 @@ #include "library/librarymodel.h" #include "library/librarywatcher.h" +#include #include FilesystemDevice::FilesystemDevice( @@ -32,35 +33,34 @@ FilesystemDevice::FilesystemDevice( int database_id, bool first_time) : FilesystemMusicStorage(url.toLocalFile()), ConnectedDevice(url, lister, unique_id, manager, app, database_id, first_time), - watcher_(new BackgroundThreadImplementation(this)) + watcher_(new LibraryWatcher), + watcher_thread_(new QThread(this)) { - // Create the library watcher - watcher_->Start(true); - watcher_->Worker()->set_device_name(manager->data(manager->index( - manager->FindDeviceById(unique_id)), DeviceManager::Role_FriendlyName).toString()); - watcher_->Worker()->set_backend(backend_); - watcher_->Worker()->set_task_manager(app_->task_manager()); + watcher_->moveToThread(watcher_thread_); + watcher_thread_->start(QThread::IdlePriority); - // To make the connections below less verbose - LibraryWatcher* watcher = watcher_->Worker().get(); + watcher_->set_device_name(manager->data(manager->index( + manager->FindDeviceById(unique_id)), DeviceManager::Role_FriendlyName).toString()); + watcher_->set_backend(backend_); + watcher_->set_task_manager(app_->task_manager()); connect(backend_, SIGNAL(DirectoryDiscovered(Directory,SubdirectoryList)), - watcher, SLOT(AddDirectory(Directory,SubdirectoryList))); + watcher_, SLOT(AddDirectory(Directory,SubdirectoryList))); connect(backend_, SIGNAL(DirectoryDeleted(Directory)), - watcher, SLOT(RemoveDirectory(Directory))); - connect(watcher, SIGNAL(NewOrUpdatedSongs(SongList)), + watcher_, SLOT(RemoveDirectory(Directory))); + connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_, SLOT(AddOrUpdateSongs(SongList))); - connect(watcher, SIGNAL(SongsMTimeUpdated(SongList)), + connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_, SLOT(UpdateMTimesOnly(SongList))); - connect(watcher, SIGNAL(SongsDeleted(SongList)), + connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_, SLOT(DeleteSongs(SongList))); - connect(watcher, SIGNAL(SubdirsDiscovered(SubdirectoryList)), + connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList))); - connect(watcher, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), + connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList))); - connect(watcher, SIGNAL(CompilationsNeedUpdating()), + connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_, SLOT(UpdateCompilations())); - connect(watcher, SIGNAL(ScanStarted(int)), SIGNAL(TaskStarted(int))); + connect(watcher_, SIGNAL(ScanStarted(int)), SIGNAL(TaskStarted(int))); } void FilesystemDevice::Init() { @@ -69,4 +69,7 @@ void FilesystemDevice::Init() { } FilesystemDevice::~FilesystemDevice() { + watcher_->deleteLater(); + watcher_thread_->exit(); + watcher_thread_->wait(); } diff --git a/src/devices/filesystemdevice.h b/src/devices/filesystemdevice.h index 4e9129843..f56d32e81 100644 --- a/src/devices/filesystemdevice.h +++ b/src/devices/filesystemdevice.h @@ -19,7 +19,6 @@ #define FILESYSTEMDEVICE_H #include "connecteddevice.h" -#include "core/backgroundthread.h" #include "core/filesystemmusicstorage.h" class DeviceManager; @@ -41,7 +40,8 @@ public: static QStringList url_schemes() { return QStringList() << "file"; } private: - BackgroundThread* watcher_; + LibraryWatcher* watcher_; + QThread* watcher_thread_; }; #endif // FILESYSTEMDEVICE_H diff --git a/src/globalsearch/globalsearch.cpp b/src/globalsearch/globalsearch.cpp index 14476a7a4..dcc444f0e 100644 --- a/src/globalsearch/globalsearch.cpp +++ b/src/globalsearch/globalsearch.cpp @@ -24,6 +24,7 @@ #include #include +#include #include const int GlobalSearch::kDelayedSearchTimeoutMs = 200; diff --git a/src/globalsearch/groovesharksearchprovider.h b/src/globalsearch/groovesharksearchprovider.h index 854e90b74..e0c4d23c6 100644 --- a/src/globalsearch/groovesharksearchprovider.h +++ b/src/globalsearch/groovesharksearchprovider.h @@ -19,7 +19,6 @@ #define GROOVESHARKSEARCHPROVIDER_H #include "searchprovider.h" -#include "core/backgroundthread.h" #include "covers/albumcoverloaderoptions.h" class AlbumCoverLoader; diff --git a/src/internet/internetmodel.h b/src/internet/internetmodel.h index 7463f01f5..0c1578e83 100644 --- a/src/internet/internetmodel.h +++ b/src/internet/internetmodel.h @@ -18,7 +18,6 @@ #ifndef INTERNETMODEL_H #define INTERNETMODEL_H -#include "core/backgroundthread.h" #include "core/song.h" #include "library/librarymodel.h" #include "playlist/playlistitem.h" diff --git a/src/internet/spotifyblobdownloader.cpp b/src/internet/spotifyblobdownloader.cpp index ebd6cd5be..16cbcdb75 100644 --- a/src/internet/spotifyblobdownloader.cpp +++ b/src/internet/spotifyblobdownloader.cpp @@ -22,6 +22,7 @@ #include "core/network.h" #include "core/utilities.h" +#include #include #include #include diff --git a/src/library/library.cpp b/src/library/library.cpp index 830366087..01e853fd4 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -25,6 +25,8 @@ #include "smartplaylists/querygenerator.h" #include "smartplaylists/search.h" +#include + const char* Library::kSongsTable = "songs"; const char* Library::kDirsTable = "directories"; const char* Library::kSubdirsTable = "subdirectories"; @@ -35,8 +37,8 @@ Library::Library(Application* app, QObject *parent) app_(app), backend_(NULL), model_(NULL), - watcher_factory_(new BackgroundThreadFactoryImplementation), - watcher_(NULL) + watcher_(NULL), + watcher_thread_(NULL) { backend_ = new LibraryBackend; backend()->moveToThread(app->database()->thread()); @@ -96,83 +98,65 @@ Library::Library(Application* app, QObject *parent) full_rescan_revisions_[26] = tr("CUE sheet support"); } -void Library::set_watcher_factory(BackgroundThreadFactory* factory) { - watcher_factory_.reset(factory); +Library::~Library() { + watcher_->deleteLater(); + watcher_thread_->exit(); + watcher_thread_->wait(); } void Library::Init() { - watcher_ = watcher_factory_->GetThread(this); - connect(watcher_, SIGNAL(Initialised()), SLOT(WatcherInitialised())); -} + watcher_ = new LibraryWatcher; + watcher_thread_ = new QThread(this); -void Library::StartThreads() { - Q_ASSERT(watcher_); + watcher_->moveToThread(watcher_thread_); + watcher_thread_->start(QThread::IdlePriority); - watcher_->set_io_priority(BackgroundThreadBase::IOPRIO_CLASS_IDLE); - watcher_->set_cpu_priority(QThread::IdlePriority); - watcher_->Start(); - - model_->Init(); -} - -void Library::WatcherInitialised() { - LibraryWatcher* watcher = watcher_->Worker().get(); - - watcher->set_backend(backend_); - watcher->set_task_manager(app_->task_manager()); + watcher_->set_backend(backend_); + watcher_->set_task_manager(app_->task_manager()); connect(backend_, SIGNAL(DirectoryDiscovered(Directory,SubdirectoryList)), - watcher, SLOT(AddDirectory(Directory,SubdirectoryList))); + watcher_, SLOT(AddDirectory(Directory,SubdirectoryList))); connect(backend_, SIGNAL(DirectoryDeleted(Directory)), - watcher, SLOT(RemoveDirectory(Directory))); - connect(watcher, SIGNAL(NewOrUpdatedSongs(SongList)), + watcher_, SLOT(RemoveDirectory(Directory))); + connect(watcher_, SIGNAL(NewOrUpdatedSongs(SongList)), backend_, SLOT(AddOrUpdateSongs(SongList))); - connect(watcher, SIGNAL(SongsMTimeUpdated(SongList)), + connect(watcher_, SIGNAL(SongsMTimeUpdated(SongList)), backend_, SLOT(UpdateMTimesOnly(SongList))); - connect(watcher, SIGNAL(SongsDeleted(SongList)), + connect(watcher_, SIGNAL(SongsDeleted(SongList)), backend_, SLOT(MarkSongsUnavailable(SongList))); - connect(watcher, SIGNAL(SubdirsDiscovered(SubdirectoryList)), + connect(watcher_, SIGNAL(SubdirsDiscovered(SubdirectoryList)), backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList))); - connect(watcher, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), + connect(watcher_, SIGNAL(SubdirsMTimeUpdated(SubdirectoryList)), backend_, SLOT(AddOrUpdateSubdirs(SubdirectoryList))); - connect(watcher, SIGNAL(CompilationsNeedUpdating()), + connect(watcher_, SIGNAL(CompilationsNeedUpdating()), backend_, SLOT(UpdateCompilations())); // This will start the watcher checking for updates backend_->LoadDirectoriesAsync(); } -void Library::IncrementalScan() { - if (!watcher_->Worker()) - return; +void Library::StartThreads() { + Q_ASSERT(watcher_); - watcher_->Worker()->IncrementalScanAsync(); + model_->Init(); +} + +void Library::IncrementalScan() { + watcher_->IncrementalScanAsync(); } void Library::FullScan() { - if (!watcher_->Worker()) - return; - - watcher_->Worker()->FullScanAsync(); + watcher_->FullScanAsync(); } void Library::PauseWatcher() { - if (!watcher_->Worker()) - return; - - watcher_->Worker()->SetRescanPausedAsync(true); + watcher_->SetRescanPausedAsync(true); } void Library::ResumeWatcher() { - if (!watcher_->Worker()) - return; - - watcher_->Worker()->SetRescanPausedAsync(false); + watcher_->SetRescanPausedAsync(false); } void Library::ReloadSettings() { - if (!watcher_->Worker()) - return; - - watcher_->Worker()->ReloadSettingsAsync(); + watcher_->ReloadSettingsAsync(); } diff --git a/src/library/library.h b/src/library/library.h index ba6989128..8e702fb75 100644 --- a/src/library/library.h +++ b/src/library/library.h @@ -18,8 +18,7 @@ #ifndef LIBRARY_H #define LIBRARY_H -#include "core/backgroundthread.h" - +#include #include #include @@ -36,15 +35,13 @@ class Library : public QObject { public: Library(Application* app, QObject* parent); + ~Library(); static const char* kSongsTable; static const char* kDirsTable; static const char* kSubdirsTable; static const char* kFtsTable; - // Useful for tests. The library takes ownership. - void set_watcher_factory(BackgroundThreadFactory* factory); - void Init(); void StartThreads(); @@ -63,15 +60,14 @@ class Library : public QObject { private slots: void IncrementalScan(); - void WatcherInitialised(); private: Application* app_; LibraryBackend* backend_; LibraryModel* model_; - boost::scoped_ptr > watcher_factory_; - BackgroundThread* watcher_; + LibraryWatcher* watcher_; + QThread* watcher_thread_; // DB schema versions which should trigger a full library rescan (each of those with // a short reason why). diff --git a/src/library/librarywatcher.cpp b/src/library/librarywatcher.cpp index 83d7507ab..3d95b33bf 100644 --- a/src/library/librarywatcher.cpp +++ b/src/library/librarywatcher.cpp @@ -22,6 +22,7 @@ #include "core/logging.h" #include "core/tagreaderclient.h" #include "core/taskmanager.h" +#include "core/utilities.h" #include "playlistparsers/cueparser.h" #include @@ -59,6 +60,8 @@ LibraryWatcher::LibraryWatcher(QObject* parent) total_watches_(0), cue_parser_(new CueParser(backend_, this)) { + Utilities::SetThreadIOPriority(Utilities::IOPRIO_CLASS_IDLE); + rescan_timer_->setInterval(1000); rescan_timer_->setSingleShot(true);