Merge pull request #15678 from unknownbrackets/clean-recent-thread

Clean recent ISOs on a thread

Core/Config.cpp

@@ -19,8 +19,10 @@
 #include <cstdlib>
 #include <ctime>
 #include <functional>
+#include <mutex>
 #include <set>
 #include <sstream>
+#include <thread>
 
 #include "ppsspp_config.h"
 
@@ -57,6 +59,17 @@ Config g_Config;
 
 bool jitForcedOff;
 
+// 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<void()> f);
+};
+
 #ifdef _DEBUG
 static const char *logSectionName = "LogDebug";
 #else
@@ -1206,13 +1219,30 @@ static void IterateSettings(IniFile &iniFile, std::function<void(Section *sectio
 	}
 }
 
+void ConfigPrivate::ResetRecentIsosThread() {
+	std::lock_guard<std::mutex> guard(recentIsosThreadLock);
+	if (recentIsosThreadPending && recentIsosThread.joinable())
+		recentIsosThread.join();
+}
+
+void ConfigPrivate::SetRecentIsosThread(std::function<void()> f) {
+	std::lock_guard<std::mutex> guard(recentIsosThreadLock);
+	if (recentIsosThreadPending && recentIsosThread.joinable())
+		recentIsosThread.join();
+	recentIsosThread = std::thread(f);
+	recentIsosThreadPending = true;
+}
+
 Config::Config() {
+	private_ = new ConfigPrivate();
 }
 
 Config::~Config() {
 	if (bUpdatedInstanceCounter) {
 		ShutdownInstanceCounter();
 	}
+	private_->ResetRecentIsosThread();
+	delete private_;
 }
 
 std::map<std::string, std::pair<std::string, int>> GetLangValuesMapping() {
@@ -1315,6 +1345,8 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) {
 		iMaxRecent = 60;
 
 	if (iMaxRecent > 0) {
+		private_->ResetRecentIsosThread();
+		std::lock_guard<std::mutex> guard(private_->recentIsosLock);
 		recentIsos.clear();
 		for (int i = 0; i < iMaxRecent; i++) {
 			char keyName[64];
@@ -1470,9 +1502,11 @@ bool Config::Save(const char *saveReason) {
 		Section *recent = iniFile.GetOrCreateSection("Recent");
 		recent->Set("MaxRecent", iMaxRecent);
 
+		private_->ResetRecentIsosThread();
 		for (int i = 0; i < iMaxRecent; i++) {
 			char keyName[64];
 			snprintf(keyName, sizeof(keyName), "FileName%d", i);
+			std::lock_guard<std::mutex> guard(private_->recentIsosLock);
 			if (i < (int)recentIsos.size()) {
 				recent->Set(keyName, recentIsos[i]);
 			} else {
@@ -1610,6 +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);
 
+	private_->ResetRecentIsosThread();
+	std::lock_guard<std::mutex> guard(private_->recentIsosLock);
 	const std::string filename = File::ResolvePath(file);
 	recentIsos.insert(recentIsos.begin(), filename);
 	if ((int)recentIsos.size() > iMaxRecent)
@@ -1621,6 +1657,8 @@ void Config::RemoveRecent(const std::string &file) {
 	if (iMaxRecent <= 0)
 		return;
 
+	private_->ResetRecentIsosThread();
+	std::lock_guard<std::mutex> 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);
@@ -1634,35 +1672,52 @@ void Config::RemoveRecent(const std::string &file) {
 }
 
 void Config::CleanRecent() {
-	double startTime = time_now_d();
-
-	std::vector<std::string> 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;
-		}
+	private_->SetRecentIsosThread([this] {
+		double startTime = time_now_d();
+
+		std::lock_guard<std::mutex> guard(private_->recentIsosLock);
+		std::vector<std::string> 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<std::string> Config::RecentIsos() const {
+	std::lock_guard<std::mutex> guard(private_->recentIsosLock);
+	return recentIsos;
+}
+bool Config::HasRecentIsos() const {
+	std::lock_guard<std::mutex> guard(private_->recentIsosLock);
+	return !recentIsos.empty();
+}
+void Config::ClearRecentIsos() {
+	private_->ResetRecentIsosThread();
+	std::lock_guard<std::mutex> guard(private_->recentIsosLock);
+	recentIsos.clear();
 }
 
 void Config::SetSearchPath(const Path &searchPath) {
@@ -1701,7 +1756,7 @@ void Config::RestoreDefaults() {
 	} else {
 		if (File::Exists(iniFilename_))
 			File::Delete(iniFilename_);
-		recentIsos.clear();
+		ClearRecentIsos();
 		currentDirectory = defaultCurrentDirectory;
 	}
 	Load();

Core/Config.h

@@ -48,6 +48,7 @@ namespace http {
 }
 
 struct UrlEncoder;
+struct ConfigPrivate;
 
 struct ConfigTouchPos {
 	float x;
@@ -140,7 +141,6 @@ struct Config {
 	int iInternalScreenRotation;  // The internal screen rotation angle. Useful for vertical SHMUPs and similar.
 
 	std::string sReportHost;
-	std::vector<std::string> recentIsos;
 	std::vector<std::string> vPinnedPaths;
 	std::string sLanguageIni;
 
@@ -536,16 +536,22 @@ struct Config {
 		return bFullScreen;
 	}
 
+	std::vector<std::string> RecentIsos() const;
+	bool HasRecentIsos() const;
+	void ClearRecentIsos();
+
 protected:
 	void LoadStandardControllerIni();
 
 private:
 	bool reload_ = false;
 	std::string gameId_;
 	std::string gameIdTitle_;
+	std::vector<std::string> recentIsos;
 	Path iniFilename_;
 	Path controllerIniFilename_;
 	Path searchPath_;
+	ConfigPrivate *private_ = nullptr;
 };
 
 std::map<std::string, std::pair<std::string, int>> GetLangValuesMapping();

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;

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());

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;
 	}

UI/MainScreen.cpp

@@ -587,7 +587,7 @@ bool GameBrowser::DisplayTopBar() {
 bool GameBrowser::HasSpecialFiles(std::vector<Path> &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;
 }

UI/MiscScreens.cpp

@@ -203,7 +203,7 @@ class RecentGamesAnimation : public Animation {
 			lastIndex_ = nextIndex_;
 		}
 
-		if (!g_Config.recentIsos.empty()) {
+		if (g_Config.HasRecentIsos()) {
 			std::shared_ptr<GameInfo> lastInfo = GetInfo(dc, lastIndex_);
 			std::shared_ptr<GameInfo> nextInfo = GetInfo(dc, nextIndex_);
 			dc.Flush();
@@ -220,12 +220,12 @@ class RecentGamesAnimation : public Animation {
 
 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 @@ class RecentGamesAnimation : public Animation {
 		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<GameInfo> &ginfo, float amount) {