From 1497685ea5d7059b34bfb65643388a4f4edf4d49 Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sat, 6 Jun 2020 18:08:30 +0200 Subject: Make checkForProblems update local variable with the problem count, Make updateProblemsButton just update the ui using that variable (no heavy operation) Add checkForProblemsAsync to perform the heavy checks in another thread. --- src/mainwindow.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 477cbc70..c49b0bdd 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -266,6 +266,7 @@ MainWindow::MainWindow(Settings &settings , m_LinkToolbar(nullptr) , m_LinkDesktop(nullptr) , m_LinkStartMenu(nullptr) + , m_NumberOfProblems(0) { // disables incredibly slow menu fade in effect that looks and feels like crap. // this was only happening to users with the windows @@ -473,7 +474,8 @@ MainWindow::MainWindow(Settings &settings setFilterShortcuts(ui->downloadView, ui->downloadFilterEdit); m_UpdateProblemsTimer.setSingleShot(true); - connect(&m_UpdateProblemsTimer, SIGNAL(timeout()), this, SLOT(updateProblemsButton())); + connect(&m_UpdateProblemsTimer, SIGNAL(timeout()), this, SLOT(checkForProblemsAsync())); + connect(this, SIGNAL(checkForProblemsDone()), this, SLOT(updateProblemsButton()), Qt::ConnectionType::QueuedConnection); m_SaveMetaTimer.setSingleShot(false); connect(&m_SaveMetaTimer, SIGNAL(timeout()), this, SLOT(saveModMetas())); @@ -1000,7 +1002,7 @@ void MainWindow::on_centralWidget_customContextMenuRequested(const QPoint &pos) createPopupMenu()->exec(ui->centralWidget->mapToGlobal(pos)); } -void MainWindow::scheduleUpdateButton() +void MainWindow::scheduleCheckForProblems() { if (!m_UpdateProblemsTimer.isActive()) { m_UpdateProblemsTimer.start(1000); @@ -1014,7 +1016,7 @@ void MainWindow::updateProblemsButton() // if the current stylesheet doesn't provide an icon, this is used instead const char* DefaultIconName = ":/MO/gui/warning"; - const std::size_t numProblems = checkForProblems(); + const std::size_t numProblems = m_NumberOfProblems; // original icon without a count painted on it const QIcon original = m_originalNotificationIcon.isNull() ? @@ -1097,8 +1099,13 @@ bool MainWindow::errorReported(QString &logFile) return false; } +void MainWindow::checkForProblemsAsync() { + QtConcurrent::run([this]() { + checkForProblems(); + }); +} -size_t MainWindow::checkForProblems() +void MainWindow::checkForProblems() { size_t numProblems = 0; for (QObject *pluginObj : m_PluginContainer.plugins()) { @@ -1109,7 +1116,8 @@ size_t MainWindow::checkForProblems() numProblems += diagnose->activeProblems().size(); } } - return numProblems; + m_NumberOfProblems = numProblems; + emit checkForProblemsDone(); } void MainWindow::about() @@ -1353,7 +1361,7 @@ void MainWindow::showEvent(QShowEvent *event) m_OrganizerCore.settings().nexus().registerAsNXMHandler(false); m_WasVisible = true; - updateProblemsButton(); + updateProblemsButton(); } } -- cgit v1.3.1 From a30ec0018bfa9056492cab100321ebfcb7b0672e Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sat, 6 Jun 2020 23:13:14 +0200 Subject: * Change the diagnoseUpdate() signals to collapse into a single async call in case of multiple signals in short time span. * Make mainwindow constructor and show() use updateProblemsButton() to just update the problems icon without performing checks. * Make directory refreshed schedule a problems check since it can overlap with other invocations. * Make changing translation not trigger checks at all. * Make changing style only update the icon instead of also performing the check. * Make MainWindow ctor, MainWindow::show()and MainWindow::resetActionIcons() just update the icon without actually performing a check. * Make clearing overwrite checkForProblems synchronously. Same before opening the notifications window and after closing it. --- src/mainwindow.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index c49b0bdd..5a56e22b 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -401,7 +401,7 @@ MainWindow::MainWindow(Settings &settings ui->bossButton->setToolTip(tr("There is no supported sort mechanism for this game. You will probably have to use a third-party tool.")); } - connect(&m_PluginContainer, SIGNAL(diagnosisUpdate()), this, SLOT(updateProblemsButton())); + connect(&m_PluginContainer, SIGNAL(diagnosisUpdate()), this, SLOT(scheduleCheckForProblems())); connect(ui->savegameList, SIGNAL(itemEntered(QListWidgetItem*)), this, SLOT(saveSelectionChanged(QListWidgetItem*))); @@ -474,8 +474,8 @@ MainWindow::MainWindow(Settings &settings setFilterShortcuts(ui->downloadView, ui->downloadFilterEdit); m_UpdateProblemsTimer.setSingleShot(true); - connect(&m_UpdateProblemsTimer, SIGNAL(timeout()), this, SLOT(checkForProblemsAsync())); - connect(this, SIGNAL(checkForProblemsDone()), this, SLOT(updateProblemsButton()), Qt::ConnectionType::QueuedConnection); + connect(&m_UpdateProblemsTimer, &QTimer::timeout, this, &MainWindow::checkForProblemsAsync); + connect(this, &MainWindow::checkForProblemsDone, this, &MainWindow::updateProblemsButton, Qt::ConnectionType::QueuedConnection); m_SaveMetaTimer.setSingleShot(false); connect(&m_SaveMetaTimer, SIGNAL(timeout()), this, SLOT(saveModMetas())); @@ -1011,8 +1011,6 @@ void MainWindow::scheduleCheckForProblems() void MainWindow::updateProblemsButton() { - TimeThis tt("MainWindow::updateProblemsButton()"); - // if the current stylesheet doesn't provide an icon, this is used instead const char* DefaultIconName = ":/MO/gui/warning"; @@ -1107,6 +1105,7 @@ void MainWindow::checkForProblemsAsync() { void MainWindow::checkForProblems() { + TimeThis tt("MainWindow::checkForProblems()"); size_t numProblems = 0; for (QObject *pluginObj : m_PluginContainer.plugins()) { IPlugin *plugin = qobject_cast(pluginObj); @@ -2464,7 +2463,7 @@ void MainWindow::directory_refreshed() { // some problem-reports may rely on the virtual directory tree so they need to be updated // now - updateProblemsButton(); + scheduleCheckForProblems(); //Some better check for the current tab is needed. if (ui->tabWidget->currentIndex() == 2) { @@ -3790,7 +3789,7 @@ void MainWindow::clearOverwrite() for (auto f : overwriteDir.entryList(QDir::AllDirs | QDir::Files | QDir::NoDotAndDotDot)) delList.push_back(overwriteDir.absoluteFilePath(f)); if (shellDelete(delList, true)) { - updateProblemsButton(); + checkForProblems(); m_OrganizerCore.refreshModList(); } else { const auto e = GetLastError(); @@ -5170,7 +5169,6 @@ void MainWindow::languageChange(const QString &newLanguage) createHelpMenu(); updateDownloadView(); - updateProblemsButton(); QMenu *listOptionsMenu = new QMenu(ui->listOptionsBtn); initModListContextMenu(listOptionsMenu); @@ -5877,12 +5875,12 @@ void MainWindow::on_bsaList_itemChanged(QTreeWidgetItem*, int) void MainWindow::on_actionNotifications_triggered() { - updateProblemsButton(); + checkForProblems(); ProblemsDialog problems(m_PluginContainer.plugins(), this); problems.exec(); - updateProblemsButton(); + checkForProblems(); } void MainWindow::on_actionChange_Game_triggered() -- cgit v1.3.1 From 2b4c0288b4c8315a311ffb74e9c1088ce2ad8543 Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sun, 7 Jun 2020 14:32:54 +0200 Subject: Make a couple more calls async since there is no good reason not to. --- src/mainwindow.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 5a56e22b..0bada397 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3789,7 +3789,7 @@ void MainWindow::clearOverwrite() for (auto f : overwriteDir.entryList(QDir::AllDirs | QDir::Files | QDir::NoDotAndDotDot)) delList.push_back(overwriteDir.absoluteFilePath(f)); if (shellDelete(delList, true)) { - checkForProblems(); + checkForProblemsAsync(); m_OrganizerCore.refreshModList(); } else { const auto e = GetLastError(); @@ -5880,7 +5880,7 @@ void MainWindow::on_actionNotifications_triggered() ProblemsDialog problems(m_PluginContainer.plugins(), this); problems.exec(); - checkForProblems(); + checkForProblemsAsync(); } void MainWindow::on_actionChange_Game_triggered() -- cgit v1.3.1 From 0dc66e7ff4e4561d7b7f9ffbc3158a0e1bf5a2ec Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sun, 7 Jun 2020 16:42:18 +0200 Subject: Avoid executing Problems checks concurrently. Reduce timer to 500ms. Make other calls also scheduled. --- src/mainwindow.cpp | 17 ++++++++++++++--- src/mainwindow.h | 2 ++ 2 files changed, 16 insertions(+), 3 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 0bada397..9559583b 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -267,6 +267,7 @@ MainWindow::MainWindow(Settings &settings , m_LinkDesktop(nullptr) , m_LinkStartMenu(nullptr) , m_NumberOfProblems(0) + , m_CheckingForProblems(false) { // disables incredibly slow menu fade in effect that looks and feels like crap. // this was only happening to users with the windows @@ -1005,7 +1006,7 @@ void MainWindow::on_centralWidget_customContextMenuRequested(const QPoint &pos) void MainWindow::scheduleCheckForProblems() { if (!m_UpdateProblemsTimer.isActive()) { - m_UpdateProblemsTimer.start(1000); + m_UpdateProblemsTimer.start(500); } } @@ -1106,6 +1107,15 @@ void MainWindow::checkForProblemsAsync() { void MainWindow::checkForProblems() { TimeThis tt("MainWindow::checkForProblems()"); + { + QMutexLocker lk(&m_CheckForProblemsMutex); + if (m_CheckingForProblems) { + return; + } + else { + m_CheckingForProblems = true; + } + } size_t numProblems = 0; for (QObject *pluginObj : m_PluginContainer.plugins()) { IPlugin *plugin = qobject_cast(pluginObj); @@ -1116,6 +1126,7 @@ void MainWindow::checkForProblems() } } m_NumberOfProblems = numProblems; + m_CheckingForProblems = false; emit checkForProblemsDone(); } @@ -3789,7 +3800,7 @@ void MainWindow::clearOverwrite() for (auto f : overwriteDir.entryList(QDir::AllDirs | QDir::Files | QDir::NoDotAndDotDot)) delList.push_back(overwriteDir.absoluteFilePath(f)); if (shellDelete(delList, true)) { - checkForProblemsAsync(); + scheduleCheckForProblems(); m_OrganizerCore.refreshModList(); } else { const auto e = GetLastError(); @@ -5880,7 +5891,7 @@ void MainWindow::on_actionNotifications_triggered() ProblemsDialog problems(m_PluginContainer.plugins(), this); problems.exec(); - checkForProblemsAsync(); + scheduleCheckForProblems(); } void MainWindow::on_actionChange_Game_triggered() diff --git a/src/mainwindow.h b/src/mainwindow.h index 3c0647e0..ac172474 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -360,6 +360,8 @@ private: QIcon m_originalNotificationIcon; std::atomic m_NumberOfProblems; + std::atomic m_CheckingForProblems; + QMutex m_CheckForProblemsMutex; Executable* getSelectedExecutable(); -- cgit v1.3.1 From d1c42e69c1819def3098bfac0fd96f2eb39226e9 Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sun, 7 Jun 2020 17:44:06 +0200 Subject: Make even the last direct call to checkForProblemsImpl use the async version. --- src/mainwindow.cpp | 14 ++++++++------ src/mainwindow.h | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 9559583b..4a5175e6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1098,15 +1098,15 @@ bool MainWindow::errorReported(QString &logFile) return false; } -void MainWindow::checkForProblemsAsync() { - QtConcurrent::run([this]() { - checkForProblems(); +QFuture MainWindow::checkForProblemsAsync() { + return QtConcurrent::run([this]() { + checkForProblemsImpl(); }); } -void MainWindow::checkForProblems() +void MainWindow::checkForProblemsImpl() { - TimeThis tt("MainWindow::checkForProblems()"); + TimeThis tt("MainWindow::checkForProblemsImpl()"); { QMutexLocker lk(&m_CheckForProblemsMutex); if (m_CheckingForProblems) { @@ -5886,7 +5886,9 @@ void MainWindow::on_bsaList_itemChanged(QTreeWidgetItem*, int) void MainWindow::on_actionNotifications_triggered() { - checkForProblems(); + auto future = checkForProblemsAsync(); + + future.waitForFinished(); ProblemsDialog problems(m_PluginContainer.plugins(), this); problems.exec(); diff --git a/src/mainwindow.h b/src/mainwindow.h index ac172474..14987c0d 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -248,7 +248,7 @@ private: bool extractProgress(QProgressDialog &extractProgress, int percentage, std::string fileName); // Performs checks, sets the m_NumberOfProblems and signals checkForProblemsDone(). - void checkForProblems(); + void checkForProblemsImpl(); void setCategoryListVisible(bool visible); @@ -535,7 +535,7 @@ private slots: void scheduleCheckForProblems(); // Perform the actual problem check in another thread. - void checkForProblemsAsync(); + QFuture checkForProblemsAsync(); void saveModMetas(); -- cgit v1.3.1 From d845fe9a74c9e0943defe5025b81c017a3fab41b Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sun, 7 Jun 2020 18:51:02 +0200 Subject: Make checks that occur at the same time not return immediately to avoid issues with stuff that might need to rely on check completion. --- src/mainwindow.cpp | 39 +++++++++++++++++++-------------------- src/mainwindow.h | 4 ++-- 2 files changed, 21 insertions(+), 22 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4a5175e6..1a868eec 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -267,7 +267,7 @@ MainWindow::MainWindow(Settings &settings , m_LinkDesktop(nullptr) , m_LinkStartMenu(nullptr) , m_NumberOfProblems(0) - , m_CheckingForProblems(false) + , m_ProblemsCheckRequired(false) { // disables incredibly slow menu fade in effect that looks and feels like crap. // this was only happening to users with the windows @@ -1107,27 +1107,26 @@ QFuture MainWindow::checkForProblemsAsync() { void MainWindow::checkForProblemsImpl() { TimeThis tt("MainWindow::checkForProblemsImpl()"); - { - QMutexLocker lk(&m_CheckForProblemsMutex); - if (m_CheckingForProblems) { - return; - } - else { - m_CheckingForProblems = true; - } - } - size_t numProblems = 0; - for (QObject *pluginObj : m_PluginContainer.plugins()) { - IPlugin *plugin = qobject_cast(pluginObj); - if (plugin == nullptr || plugin->isActive()) { - IPluginDiagnose *diagnose = qobject_cast(pluginObj); - if (diagnose != nullptr) - numProblems += diagnose->activeProblems().size(); + + m_ProblemsCheckRequired = true; + + std::scoped_lock lk(m_CheckForProblemsMutex); + + // another thread might already have checked while this one was waiting on the lock + if (m_ProblemsCheckRequired) { + m_ProblemsCheckRequired = false; + size_t numProblems = 0; + for (QObject *pluginObj : m_PluginContainer.plugins()) { + IPlugin *plugin = qobject_cast(pluginObj); + if (plugin == nullptr || plugin->isActive()) { + IPluginDiagnose *diagnose = qobject_cast(pluginObj); + if (diagnose != nullptr) + numProblems += diagnose->activeProblems().size(); + } } + m_NumberOfProblems = numProblems; + emit checkForProblemsDone(); } - m_NumberOfProblems = numProblems; - m_CheckingForProblems = false; - emit checkForProblemsDone(); } void MainWindow::about() diff --git a/src/mainwindow.h b/src/mainwindow.h index 14987c0d..94626ff3 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -360,8 +360,8 @@ private: QIcon m_originalNotificationIcon; std::atomic m_NumberOfProblems; - std::atomic m_CheckingForProblems; - QMutex m_CheckForProblemsMutex; + std::atomic m_ProblemsCheckRequired; + std::mutex m_CheckForProblemsMutex; Executable* getSelectedExecutable(); -- cgit v1.3.1 From 6d7c0866859a0fe27a8055068552470b67d680e3 Mon Sep 17 00:00:00 2001 From: AL <26797547+Al12rs@users.noreply.github.com> Date: Sun, 7 Jun 2020 21:06:33 +0200 Subject: Change time measurement to not include lock wait time. --- src/mainwindow.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 1a868eec..c3a19bf4 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1106,8 +1106,6 @@ QFuture MainWindow::checkForProblemsAsync() { void MainWindow::checkForProblemsImpl() { - TimeThis tt("MainWindow::checkForProblemsImpl()"); - m_ProblemsCheckRequired = true; std::scoped_lock lk(m_CheckForProblemsMutex); @@ -1115,6 +1113,7 @@ void MainWindow::checkForProblemsImpl() // another thread might already have checked while this one was waiting on the lock if (m_ProblemsCheckRequired) { m_ProblemsCheckRequired = false; + TimeThis tt("MainWindow::checkForProblemsImpl()"); size_t numProblems = 0; for (QObject *pluginObj : m_PluginContainer.plugins()) { IPlugin *plugin = qobject_cast(pluginObj); -- cgit v1.3.1