From aad5082d555039747cef9956c08344e2733c2392 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Mon, 11 Jul 2022 20:44:13 -0700 Subject: [PATCH 1/3] Config: Move recent ISOs access to accessors. --- Core/Config.cpp | 2 +- Core/Config.h | 12 +++++++++++- Core/Util/GameManager.cpp | 2 +- Core/WebServer.cpp | 4 ++-- UI/GameScreen.cpp | 4 ++-- UI/MainScreen.cpp | 10 +++++----- UI/MiscScreens.cpp | 11 +++++++---- 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index fe03875ba8..bbb534fbc6 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -1701,7 +1701,7 @@ void Config::RestoreDefaults() { } else { if (File::Exists(iniFilename_)) File::Delete(iniFilename_); - recentIsos.clear(); + ClearRecentIsos(); currentDirectory = defaultCurrentDirectory; } Load(); diff --git a/Core/Config.h b/Core/Config.h index 88aefb94e6..599c7de818 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -140,7 +140,6 @@ public: int iInternalScreenRotation; // The internal screen rotation angle. Useful for vertical SHMUPs and similar. std::string sReportHost; - std::vector recentIsos; std::vector vPinnedPaths; std::string sLanguageIni; @@ -536,6 +535,16 @@ public: return bFullScreen; } + std::vector RecentIsos() const { + return recentIsos; + } + bool HasRecentIsos() const { + return !recentIsos.empty(); + } + void ClearRecentIsos() { + recentIsos.clear(); + } + protected: void LoadStandardControllerIni(); @@ -543,6 +552,7 @@ private: bool reload_ = false; std::string gameId_; std::string gameIdTitle_; + std::vector recentIsos; Path iniFilename_; Path controllerIniFilename_; Path searchPath_; diff --git a/Core/Util/GameManager.cpp b/Core/Util/GameManager.cpp index bda6741c57..b6f4ccbd65 100644 --- a/Core/Util/GameManager.cpp +++ b/Core/Util/GameManager.cpp @@ -375,7 +375,7 @@ bool GameManager::DetectTexturePackDest(struct zip *z, int iniIndex, Path &dest) std::string gameID = games.begin()->first; if (games.size() > 1) { // Check for any supported game on their recent list and use that instead. - for (const std::string &path : g_Config.recentIsos) { + for (const std::string &path : g_Config.RecentIsos()) { std::string recentID = GetGameID(Path(path)); if (games.find(recentID) != games.end()) { gameID = recentID; diff --git a/Core/WebServer.cpp b/Core/WebServer.cpp index a55b298ed2..ff21b02a2a 100644 --- a/Core/WebServer.cpp +++ b/Core/WebServer.cpp @@ -150,7 +150,7 @@ static std::string RemotePathForRecent(const std::string &filename) { } static Path LocalFromRemotePath(const std::string &path) { - for (const std::string &filename : g_Config.recentIsos) { + for (const std::string &filename : g_Config.RecentIsos()) { std::string basename = RemotePathForRecent(filename); if (basename == path) { return Path(filename); @@ -216,7 +216,7 @@ static void HandleListing(const http::Request &request) { request.Out()->Printf("/\n"); if (serverFlags & (int)WebServerFlags::DISCS) { // List the current discs in their recent order. - for (const std::string &filename : g_Config.recentIsos) { + for (const std::string &filename : g_Config.RecentIsos()) { std::string basename = RemotePathForRecent(filename); if (!basename.empty()) { request.Out()->Printf("%s\n", basename.c_str()); diff --git a/UI/GameScreen.cpp b/UI/GameScreen.cpp index a11572941a..15d4073f98 100644 --- a/UI/GameScreen.cpp +++ b/UI/GameScreen.cpp @@ -426,8 +426,8 @@ bool GameScreen::isRecentGame(const Path &gamePath) { return false; const std::string resolved = File::ResolvePath(gamePath.ToString()); - for (auto it = g_Config.recentIsos.begin(); it != g_Config.recentIsos.end(); ++it) { - const std::string recent = File::ResolvePath(*it); + for (auto iso : g_Config.RecentIsos()) { + const std::string recent = File::ResolvePath(iso); if (resolved == recent) return true; } diff --git a/UI/MainScreen.cpp b/UI/MainScreen.cpp index b1d337908a..8b5fbb08f4 100644 --- a/UI/MainScreen.cpp +++ b/UI/MainScreen.cpp @@ -587,7 +587,7 @@ bool GameBrowser::DisplayTopBar() { bool GameBrowser::HasSpecialFiles(std::vector &filenames) { if (path_.GetPath().ToString() == "!RECENT") { filenames.clear(); - for (auto &str : g_Config.recentIsos) { + for (auto &str : g_Config.RecentIsos()) { filenames.push_back(Path(str)); } return true; @@ -986,7 +986,7 @@ void MainScreen::CreateViews() { System_GetPermissionStatus(SYSTEM_PERMISSION_STORAGE) == PERMISSION_STATUS_GRANTED; bool storageIsTemporary = IsTempPath(GetSysDirectory(DIRECTORY_SAVEDATA)) && !confirmedTemporary_; if (showRecent && !hasStorageAccess) { - showRecent = !g_Config.recentIsos.empty(); + showRecent = g_Config.HasRecentIsos(); } if (showRecent) { @@ -1036,7 +1036,7 @@ void MainScreen::CreateViews() { tabAllGames->OnHighlight.Handle(this, &MainScreen::OnGameHighlight); tabHomebrew->OnHighlight.Handle(this, &MainScreen::OnGameHighlight); - if (g_Config.recentIsos.size() > 0) { + if (g_Config.HasRecentIsos()) { tabHolder_->SetCurrentTab(0, true); } else if (g_Config.iMaxRecent > 0) { tabHolder_->SetCurrentTab(1, true); @@ -1490,7 +1490,7 @@ void UmdReplaceScreen::CreateViews() { rightColumnItems->Add(new Choice(di->T("Cancel")))->OnClick.Handle(this, &UmdReplaceScreen::OnCancel); rightColumnItems->Add(new Choice(mm->T("Game Settings")))->OnClick.Handle(this, &UmdReplaceScreen::OnGameSettings); - if (g_Config.recentIsos.size() > 0) { + if (g_Config.HasRecentIsos()) { leftColumn->SetCurrentTab(0, true); } else if (g_Config.iMaxRecent > 0) { leftColumn->SetCurrentTab(1, true); @@ -1569,7 +1569,7 @@ UI::EventReturn GridSettingsScreen::GridMinusClick(UI::EventParams &e) { } UI::EventReturn GridSettingsScreen::OnRecentClearClick(UI::EventParams &e) { - g_Config.recentIsos.clear(); + g_Config.ClearRecentIsos(); OnRecentChanged.Trigger(e); return UI::EVENT_DONE; } diff --git a/UI/MiscScreens.cpp b/UI/MiscScreens.cpp index ccd5b86919..f7e22a8f78 100644 --- a/UI/MiscScreens.cpp +++ b/UI/MiscScreens.cpp @@ -203,7 +203,7 @@ public: lastIndex_ = nextIndex_; } - if (!g_Config.recentIsos.empty()) { + if (g_Config.HasRecentIsos()) { std::shared_ptr lastInfo = GetInfo(dc, lastIndex_); std::shared_ptr nextInfo = GetInfo(dc, nextIndex_); dc.Flush(); @@ -220,12 +220,12 @@ public: private: void CheckNext(UIContext &dc, double t) { - if (g_Config.recentIsos.empty()) { + if (!g_Config.HasRecentIsos()) { return; } for (int index = lastIndex_ + 1; index != lastIndex_; ++index) { - if (index < 0 || index >= (int)g_Config.recentIsos.size()) { + if (index < 0 || index >= (int)g_Config.RecentIsos().size()) { if (lastIndex_ == -1) break; index = 0; @@ -250,7 +250,10 @@ private: if (index < 0) { return nullptr; } - return g_gameInfoCache->GetInfo(dc.GetDrawContext(), Path(g_Config.recentIsos[index]), GAMEINFO_WANTBG); + const auto recentIsos = g_Config.RecentIsos(); + if (index >= recentIsos.size()) + return nullptr; + return g_gameInfoCache->GetInfo(dc.GetDrawContext(), Path(recentIsos[index]), GAMEINFO_WANTBG); } void DrawTex(UIContext &dc, std::shared_ptr &ginfo, float amount) { From c04de9dd7976c855960affcdbd1b96726c94c1bf Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Mon, 11 Jul 2022 22:01:06 -0700 Subject: [PATCH 2/3] Config: Cleanup recent on a thread. --- Core/Config.cpp | 92 ++++++++++++++++++++++++++++++++++++------------- Core/Config.h | 12 ++----- 2 files changed, 71 insertions(+), 33 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index bbb534fbc6..ebb0521f6e 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -19,8 +19,10 @@ #include #include #include +#include #include #include +#include #include "ppsspp_config.h" @@ -56,6 +58,9 @@ http::Downloader g_DownloadManager; Config g_Config; bool jitForcedOff; +static std::mutex recentIsosLock; +static std::mutex recentIsosThreadLock; +static std::thread recentIsosThread; #ifdef _DEBUG static const char *logSectionName = "LogDebug"; @@ -1206,6 +1211,19 @@ static void IterateSettings(IniFile &iniFile, std::function guard(recentIsosThreadLock); + if (recentIsosThread.joinable()) + recentIsosThread.join(); +} + +static void SetRecentIsosThread(std::function f) { + std::lock_guard guard(recentIsosThreadLock); + if (recentIsosThread.joinable()) + recentIsosThread.join(); + recentIsosThread = std::thread(f); +} + Config::Config() { } @@ -1213,6 +1231,7 @@ Config::~Config() { if (bUpdatedInstanceCounter) { ShutdownInstanceCounter(); } + ResetRecentIsosThread(); } std::map> GetLangValuesMapping() { @@ -1315,6 +1334,8 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) { iMaxRecent = 60; if (iMaxRecent > 0) { + ResetRecentIsosThread(); + std::lock_guard guard(recentIsosLock); recentIsos.clear(); for (int i = 0; i < iMaxRecent; i++) { char keyName[64]; @@ -1470,9 +1491,11 @@ bool Config::Save(const char *saveReason) { Section *recent = iniFile.GetOrCreateSection("Recent"); recent->Set("MaxRecent", iMaxRecent); + ResetRecentIsosThread(); for (int i = 0; i < iMaxRecent; i++) { char keyName[64]; snprintf(keyName, sizeof(keyName), "FileName%d", i); + std::lock_guard guard(recentIsosLock); if (i < (int)recentIsos.size()) { recent->Set(keyName, recentIsos[i]); } else { @@ -1610,6 +1633,8 @@ void Config::AddRecent(const std::string &file) { // We'll add it back below. This makes sure it's at the front, and only once. RemoveRecent(file); + ResetRecentIsosThread(); + std::lock_guard guard(recentIsosLock); const std::string filename = File::ResolvePath(file); recentIsos.insert(recentIsos.begin(), filename); if ((int)recentIsos.size() > iMaxRecent) @@ -1621,6 +1646,8 @@ void Config::RemoveRecent(const std::string &file) { if (iMaxRecent <= 0) return; + ResetRecentIsosThread(); + std::lock_guard guard(recentIsosLock); const std::string filename = File::ResolvePath(file); for (auto iter = recentIsos.begin(); iter != recentIsos.end();) { const std::string recent = File::ResolvePath(*iter); @@ -1634,35 +1661,52 @@ void Config::RemoveRecent(const std::string &file) { } void Config::CleanRecent() { - double startTime = time_now_d(); + SetRecentIsosThread([this] { + double startTime = time_now_d(); - std::vector cleanedRecent; - for (size_t i = 0; i < recentIsos.size(); i++) { - bool exists = false; - Path path = Path(recentIsos[i]); - switch (path.Type()) { - case PathType::CONTENT_URI: - case PathType::NATIVE: - exists = File::Exists(path); - break; - default: - FileLoader *loader = ConstructFileLoader(path); - exists = loader->ExistsFast(); - delete loader; - break; - } + std::lock_guard guard(recentIsosLock); + std::vector cleanedRecent; + for (size_t i = 0; i < recentIsos.size(); i++) { + bool exists = false; + Path path = Path(recentIsos[i]); + switch (path.Type()) { + case PathType::CONTENT_URI: + case PathType::NATIVE: + exists = File::Exists(path); + break; + default: + FileLoader *loader = ConstructFileLoader(path); + exists = loader->ExistsFast(); + delete loader; + break; + } - if (exists) { - // Make sure we don't have any redundant items. - auto duplicate = std::find(cleanedRecent.begin(), cleanedRecent.end(), recentIsos[i]); - if (duplicate == cleanedRecent.end()) { - cleanedRecent.push_back(recentIsos[i]); + if (exists) { + // Make sure we don't have any redundant items. + auto duplicate = std::find(cleanedRecent.begin(), cleanedRecent.end(), recentIsos[i]); + if (duplicate == cleanedRecent.end()) { + cleanedRecent.push_back(recentIsos[i]); + } } } - } - INFO_LOG(SYSTEM, "CleanRecent took %0.2f", time_now_d() - startTime); - recentIsos = cleanedRecent; + INFO_LOG(SYSTEM, "CleanRecent took %0.2f", time_now_d() - startTime); + recentIsos = cleanedRecent; + }); +} + +std::vector Config::RecentIsos() const { + std::lock_guard guard(recentIsosLock); + return recentIsos; +} +bool Config::HasRecentIsos() const { + std::lock_guard guard(recentIsosLock); + return !recentIsos.empty(); +} +void Config::ClearRecentIsos() { + ResetRecentIsosThread(); + std::lock_guard guard(recentIsosLock); + recentIsos.clear(); } void Config::SetSearchPath(const Path &searchPath) { diff --git a/Core/Config.h b/Core/Config.h index 599c7de818..1125f5437b 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -535,15 +535,9 @@ public: return bFullScreen; } - std::vector RecentIsos() const { - return recentIsos; - } - bool HasRecentIsos() const { - return !recentIsos.empty(); - } - void ClearRecentIsos() { - recentIsos.clear(); - } + std::vector RecentIsos() const; + bool HasRecentIsos() const; + void ClearRecentIsos(); protected: void LoadStandardControllerIni(); From e9891e084364593a5c74e5f331087e412f4f3e06 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Mon, 11 Jul 2022 23:46:09 -0700 Subject: [PATCH 3/3] Config: Keep recentIsos lock owned by Config. --- Core/Config.cpp | 55 +++++++++++++++++++++++++++++-------------------- Core/Config.h | 2 ++ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index ebb0521f6e..d4753cf3e1 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -58,9 +58,17 @@ http::Downloader g_DownloadManager; Config g_Config; bool jitForcedOff; -static std::mutex recentIsosLock; -static std::mutex recentIsosThreadLock; -static std::thread recentIsosThread; + +// Not in Config.h because it's #included a lot. +struct ConfigPrivate { + std::mutex recentIsosLock; + std::mutex recentIsosThreadLock; + std::thread recentIsosThread; + bool recentIsosThreadPending = false; + + void ResetRecentIsosThread(); + void SetRecentIsosThread(std::function f); +}; #ifdef _DEBUG static const char *logSectionName = "LogDebug"; @@ -1211,27 +1219,30 @@ static void IterateSettings(IniFile &iniFile, std::function guard(recentIsosThreadLock); - if (recentIsosThread.joinable()) + if (recentIsosThreadPending && recentIsosThread.joinable()) recentIsosThread.join(); } -static void SetRecentIsosThread(std::function f) { +void ConfigPrivate::SetRecentIsosThread(std::function f) { std::lock_guard guard(recentIsosThreadLock); - if (recentIsosThread.joinable()) + if (recentIsosThreadPending && recentIsosThread.joinable()) recentIsosThread.join(); recentIsosThread = std::thread(f); + recentIsosThreadPending = true; } Config::Config() { + private_ = new ConfigPrivate(); } Config::~Config() { if (bUpdatedInstanceCounter) { ShutdownInstanceCounter(); } - ResetRecentIsosThread(); + private_->ResetRecentIsosThread(); + delete private_; } std::map> GetLangValuesMapping() { @@ -1334,8 +1345,8 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) { iMaxRecent = 60; if (iMaxRecent > 0) { - ResetRecentIsosThread(); - std::lock_guard guard(recentIsosLock); + private_->ResetRecentIsosThread(); + std::lock_guard guard(private_->recentIsosLock); recentIsos.clear(); for (int i = 0; i < iMaxRecent; i++) { char keyName[64]; @@ -1491,11 +1502,11 @@ bool Config::Save(const char *saveReason) { Section *recent = iniFile.GetOrCreateSection("Recent"); recent->Set("MaxRecent", iMaxRecent); - ResetRecentIsosThread(); + private_->ResetRecentIsosThread(); for (int i = 0; i < iMaxRecent; i++) { char keyName[64]; snprintf(keyName, sizeof(keyName), "FileName%d", i); - std::lock_guard guard(recentIsosLock); + std::lock_guard guard(private_->recentIsosLock); if (i < (int)recentIsos.size()) { recent->Set(keyName, recentIsos[i]); } else { @@ -1633,8 +1644,8 @@ void Config::AddRecent(const std::string &file) { // We'll add it back below. This makes sure it's at the front, and only once. RemoveRecent(file); - ResetRecentIsosThread(); - std::lock_guard guard(recentIsosLock); + private_->ResetRecentIsosThread(); + std::lock_guard guard(private_->recentIsosLock); const std::string filename = File::ResolvePath(file); recentIsos.insert(recentIsos.begin(), filename); if ((int)recentIsos.size() > iMaxRecent) @@ -1646,8 +1657,8 @@ void Config::RemoveRecent(const std::string &file) { if (iMaxRecent <= 0) return; - ResetRecentIsosThread(); - std::lock_guard guard(recentIsosLock); + private_->ResetRecentIsosThread(); + std::lock_guard guard(private_->recentIsosLock); const std::string filename = File::ResolvePath(file); for (auto iter = recentIsos.begin(); iter != recentIsos.end();) { const std::string recent = File::ResolvePath(*iter); @@ -1661,10 +1672,10 @@ void Config::RemoveRecent(const std::string &file) { } void Config::CleanRecent() { - SetRecentIsosThread([this] { + private_->SetRecentIsosThread([this] { double startTime = time_now_d(); - std::lock_guard guard(recentIsosLock); + std::lock_guard guard(private_->recentIsosLock); std::vector cleanedRecent; for (size_t i = 0; i < recentIsos.size(); i++) { bool exists = false; @@ -1696,16 +1707,16 @@ void Config::CleanRecent() { } std::vector Config::RecentIsos() const { - std::lock_guard guard(recentIsosLock); + std::lock_guard guard(private_->recentIsosLock); return recentIsos; } bool Config::HasRecentIsos() const { - std::lock_guard guard(recentIsosLock); + std::lock_guard guard(private_->recentIsosLock); return !recentIsos.empty(); } void Config::ClearRecentIsos() { - ResetRecentIsosThread(); - std::lock_guard guard(recentIsosLock); + private_->ResetRecentIsosThread(); + std::lock_guard guard(private_->recentIsosLock); recentIsos.clear(); } diff --git a/Core/Config.h b/Core/Config.h index 1125f5437b..e682234ba3 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -48,6 +48,7 @@ namespace http { } struct UrlEncoder; +struct ConfigPrivate; struct ConfigTouchPos { float x; @@ -550,6 +551,7 @@ private: Path iniFilename_; Path controllerIniFilename_; Path searchPath_; + ConfigPrivate *private_ = nullptr; }; std::map> GetLangValuesMapping();