Skip to content

Commit

Permalink
act on review
Browse files Browse the repository at this point in the history
  • Loading branch information
m0dB authored and m0dB committed Nov 11, 2024
1 parent 043971e commit 1aec9f9
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 53 deletions.
99 changes: 54 additions & 45 deletions src/library/browse/browsethread.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,36 @@ constexpr int kRowBatchSize = 10;
* make sense to use this class in non-GUI threads
*/
BrowseThread::BrowseThread(QObject* parent)
: QThread{parent}, m_modelObserver{}, m_bRun{} {
: QThread{parent}, m_pModelObserver{}, m_requestedRunState{} {
// Start thread
start(QThread::LowPriority);

qDebug() << "Wait to start browser background thread";
// Wait until the thread is running
m_mutex.lock();
while (!m_bRun) {
m_condition.wait(&m_mutex);
m_requestMutex.lock();
while (!m_requestedRunState) {
m_requestCondition.wait(&m_requestMutex);
}
m_mutex.unlock();
m_requestMutex.unlock();
qDebug() << "Browser background thread started";
}

BrowseThread::~BrowseThread() {
qDebug() << "Wait to terminate browser background thread";
// Inform the thread we want it to terminate
m_mutex.lock();
m_bRun = false;
m_condition.wakeOne();
m_mutex.unlock();
requestStop();
// Wait until thread terminated
wait();
qDebug() << "Browser background thread terminated";
}

void BrowseThread::requestStop() {
qDebug() << "Request to terminate browser background thread";
m_requestMutex.lock();
m_requestedRunState = false;
m_requestMutex.unlock();
m_requestCondition.wakeOne();
}

// static
BrowseThreadPointer BrowseThread::getInstanceRef() {
// We create a single BrowseThread instead which is used by multiple
Expand All @@ -60,50 +65,54 @@ BrowseThreadPointer BrowseThread::getInstanceRef() {
static QWeakPointer<BrowseThread> s_weakInstanceRef;
static QMutex s_mutex;

s_mutex.lock();
BrowseThreadPointer strong = s_weakInstanceRef.toStrongRef();

if (!strong) {
strong = BrowseThreadPointer(new BrowseThread());
s_weakInstanceRef = strong.toWeakRef();
s_mutex.lock();
strong = s_weakInstanceRef.toStrongRef();
if (!strong) {
strong = BrowseThreadPointer(new BrowseThread());
s_weakInstanceRef = strong.toWeakRef();
}
s_mutex.unlock();
}
s_mutex.unlock();

return strong;
}

void BrowseThread::requestPopulateModel(mixxx::FileAccess path, BrowseTableModel* modelObserver) {
void BrowseThread::requestPopulateModel(mixxx::FileAccess path, BrowseTableModel* pModelObserver) {
// Inform the thread to populate a new path.
// Note: if another request is currently being processed, this will replace it.

qDebug() << "Request populate model" << path.info() << modelObserver;
m_mutex.lock();
m_path = std::move(path);
m_modelObserver = modelObserver;
m_condition.wakeOne();
m_mutex.unlock();
qDebug() << "Request populate model" << path.info() << pModelObserver;
m_requestMutex.lock();
m_requestedPath = std::move(path);
m_pModelObserver = pModelObserver;
m_requestCondition.wakeOne();
m_requestMutex.unlock();
}

void BrowseThread::run() {
QThread::currentThread()->setObjectName("BrowseThread");

// Inform the constructor the thread started
m_mutex.lock();
m_bRun = true;
m_condition.wakeOne();
m_mutex.unlock();
m_requestMutex.lock();
m_requestedRunState = true;
m_requestCondition.wakeOne();
m_requestMutex.unlock();

while (true) {
// Wait for a new population request, or for a termination request
qDebug() << "Wait for a new population request";
m_mutex.lock();
while (!m_path.isSet() && m_bRun) {
m_condition.wait(&m_mutex);
m_requestMutex.lock();
while (!m_requestedPath.isSet() && m_requestedRunState) {
m_requestCondition.wait(&m_requestMutex);
}
auto path = std::move(m_path);
auto modelObserver = m_modelObserver;
auto bRun = m_bRun;
m_path = mixxx::FileAccess();
m_mutex.unlock();
auto path = std::move(m_requestedPath);
auto pModelObserver = m_pModelObserver;

Check warning on line 112 in src/library/browse/browsethread.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

'auto pModelObserver' can be declared as 'auto *pModelObserver' [readability-qualified-auto]
auto bRun = m_requestedRunState;
m_requestedPath = mixxx::FileAccess();
m_requestMutex.unlock();

Trace trace("BrowseThread");

Expand All @@ -114,7 +123,7 @@ void BrowseThread::run() {
} else {
qDebug() << "New population request";
// Populate the model
populateModel(path, modelObserver);
populateModel(path, pModelObserver);
}
}
}
Expand All @@ -141,15 +150,15 @@ class YearItem: public QStandardItem {

} // namespace

void BrowseThread::populateModel(mixxx::FileAccess path, BrowseTableModel* modelObserver) {
void BrowseThread::populateModel(mixxx::FileAccess path, BrowseTableModel* pModelObserver) {

Check failure on line 153 in src/library/browse/browsethread.cpp

View workflow job for this annotation

GitHub Actions / clazy 

Missing reference on large type (sizeof mixxx::FileAccess is 24 bytes) [-Wclazy-function-args-by-ref]
// Executed by the thread run for incoming populate model requests

if (!path.info().hasLocation()) {
// Abort if the location is inaccessible or does not exist
qWarning() << "Skipping" << path.info();
return;
}
qDebug() << "populateModel" << path.info() << modelObserver;
qDebug() << "populateModel" << path.info() << pModelObserver;

// Refresh the name filters in case we loaded new SoundSource plugins.
const QStringList nameFilters = SoundSourceProxy::getSupportedFileNamePatterns();
Expand All @@ -161,7 +170,7 @@ void BrowseThread::populateModel(mixxx::FileAccess path, BrowseTableModel* model
// remove all rows
// This is a blocking operation
// see signal/slot connection in BrowseTableModel
emit clearModel(modelObserver);
emit clearModel(pModelObserver);

QList<QList<QStandardItem*>> rows;
rows.reserve(kRowBatchSize);
Expand All @@ -175,12 +184,12 @@ void BrowseThread::populateModel(mixxx::FileAccess path, BrowseTableModel* model
// was received, we abort the current request. This happens if a user
// quickly jumps through the folders and the current task becomes
// "dirty".
m_mutex.lock();
const bool abortProcess = !m_bRun ||
(m_path.isSet() &&
(m_path.info() != path.info() ||
modelObserver != m_modelObserver));
m_mutex.unlock();
m_requestMutex.lock();
const bool abortProcess = !m_requestedRunState ||
(m_requestedPath.isSet() &&
(m_requestedPath.info() != path.info() ||
m_pModelObserver != pModelObserver));
m_requestMutex.unlock();
if (abortProcess) {
qDebug() << "Abort populateModel";
// The run function will take care of the new request.
Expand Down Expand Up @@ -333,14 +342,14 @@ void BrowseThread::populateModel(mixxx::FileAccess path, BrowseTableModel* model
// Will limit GUI freezing
if (row % kRowBatchSize == 0) {
// this is a blocking operation
emit rowsAppended(rows, modelObserver);
emit rowsAppended(rows, pModelObserver);
qDebug() << "Append" << rows.count() << "tracks from "
<< path.info().locationPath();
rows.clear();
}
// Sleep additionally for 20ms which prevents us from GUI freezes
msleep(20);
}
emit rowsAppended(rows, modelObserver);
emit rowsAppended(rows, pModelObserver);
qDebug() << "Append last" << rows.count() << "tracks from" << path.info().locationPath();
}
18 changes: 10 additions & 8 deletions src/library/browse/browsethread.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class BrowseThread : public QThread {
Q_OBJECT
public:
virtual ~BrowseThread();
void requestPopulateModel(mixxx::FileAccess path, BrowseTableModel* modelObserver);
void requestPopulateModel(mixxx::FileAccess path, BrowseTableModel* pModelObserver);
void run();
static BrowseThreadPointer getInstanceRef();

Expand All @@ -41,13 +41,15 @@ class BrowseThread : public QThread {
private:
BrowseThread(QObject *parent = 0);

void populateModel(mixxx::FileAccess path, BrowseTableModel* modelObserver);
void requestStop();
void populateModel(mixxx::FileAccess path, BrowseTableModel* pModelObserver);

QMutex m_mutex;
QWaitCondition m_condition;
QMutex m_requestMutex;
QWaitCondition m_requestCondition;

// You must hold m_mutex to touch m_path, m_modelObserver or m_bRun
mixxx::FileAccess m_path;
BrowseTableModel* m_modelObserver;
bool m_bRun;
// You must hold m_requestMutex to touch m_requestedPath, m_pModelObserver
// or m_requestedRunState
mixxx::FileAccess m_requestedPath;
BrowseTableModel* m_pModelObserver;
bool m_requestedRunState;
};

0 comments on commit 1aec9f9

Please sign in to comment.