From 04cefc6ed343a1c20ed5b54e25f06d6b43ba3fbe Mon Sep 17 00:00:00 2001 From: JosJuice Date: Thu, 4 Jan 2018 17:10:25 +0100 Subject: [PATCH 1/2] DolphinWX: Use vector instead of list for game list cache The advantage of std::list is that elements can be removed from the middle efficiently, but we don't actually need that, because the ordering of the elements doesn't matter for us. We can just replace the element we want to remove with the last element and then call pop_back. Replacing list with vector should speed up looping through the elements. --- Source/Core/DolphinWX/GameListCtrl.cpp | 11 +++++++---- Source/Core/DolphinWX/GameListCtrl.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Source/Core/DolphinWX/GameListCtrl.cpp b/Source/Core/DolphinWX/GameListCtrl.cpp index ca3a4ad075..5846234941 100644 --- a/Source/Core/DolphinWX/GameListCtrl.cpp +++ b/Source/Core/DolphinWX/GameListCtrl.cpp @@ -774,7 +774,7 @@ void GameListCtrl::RescanList() cached_paths.emplace_back(file->GetFileName()); std::sort(cached_paths.begin(), cached_paths.end()); - std::list removed_paths; + std::vector removed_paths; std::set_difference(cached_paths.cbegin(), cached_paths.cend(), search_results.cbegin(), search_results.cend(), std::back_inserter(removed_paths)); @@ -795,14 +795,17 @@ void GameListCtrl::RescanList() std::unique_lock lk(m_cache_mutex); for (const auto& path : removed_paths) { - auto it = std::find_if(m_cached_files.cbegin(), m_cached_files.cend(), + auto it = std::find_if(m_cached_files.begin(), m_cached_files.end(), [&path](const std::shared_ptr& file) { return file->GetFileName() == path; }); - if (it != m_cached_files.cend()) + if (it != m_cached_files.end()) { cache_changed = true; - m_cached_files.erase(it); + + // Efficiently remove the file without caring about preserving any order + *it = std::move(m_cached_files.back()); + m_cached_files.pop_back(); } } for (const auto& path : new_paths) diff --git a/Source/Core/DolphinWX/GameListCtrl.h b/Source/Core/DolphinWX/GameListCtrl.h index 3004ce2e8a..0f39eb3b6d 100644 --- a/Source/Core/DolphinWX/GameListCtrl.h +++ b/Source/Core/DolphinWX/GameListCtrl.h @@ -125,7 +125,7 @@ private: } m_image_indexes; // Actual backing GameListItems are maintained in a background thread and cached to file - std::list> m_cached_files; + std::vector> m_cached_files; // Locks the list, not the contents std::mutex m_cache_mutex; Core::TitleDatabase m_title_database; From af51063a9a357074e9cec4ffee6b8ac9f7a8cf52 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 5 Jan 2018 18:15:40 +0100 Subject: [PATCH 2/2] DolphinWX: Rewrite the logic for adding/removing games from cache Thanks to degasus for coming up with most of this faster design. --- Source/Core/DolphinWX/GameListCtrl.cpp | 69 +++++++++++++------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/Source/Core/DolphinWX/GameListCtrl.cpp b/Source/Core/DolphinWX/GameListCtrl.cpp index 5846234941..99e66ba21e 100644 --- a/Source/Core/DolphinWX/GameListCtrl.cpp +++ b/Source/Core/DolphinWX/GameListCtrl.cpp @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -760,27 +760,20 @@ void GameListCtrl::RescanList() const std::vector search_extensions = {".gcm", ".tgc", ".iso", ".ciso", ".gcz", ".wbfs", ".wad", ".dol", ".elf"}; // TODO This could process paths iteratively as they are found - auto search_results = Common::DoFileSearch(SConfig::GetInstance().m_ISOFolder, search_extensions, - SConfig::GetInstance().m_RecursiveISOFolder); + const std::vector search_results_vector = + Common::DoFileSearch(SConfig::GetInstance().m_ISOFolder, search_extensions, + SConfig::GetInstance().m_RecursiveISOFolder); + // Copy search results into a set, except ones that match DiscIO::ShouldHideFromGameList. // TODO Prevent DoFileSearch from looking inside /files/ directories of DirectoryBlobs at all? // TODO Make DoFileSearch support filter predicates so we don't have remove things afterwards? - search_results.erase( - std::remove_if(search_results.begin(), search_results.end(), DiscIO::ShouldHideFromGameList), - search_results.end()); - - std::vector cached_paths; - for (const auto& file : m_cached_files) - cached_paths.emplace_back(file->GetFileName()); - std::sort(cached_paths.begin(), cached_paths.end()); - - std::vector removed_paths; - std::set_difference(cached_paths.cbegin(), cached_paths.cend(), search_results.cbegin(), - search_results.cend(), std::back_inserter(removed_paths)); - - std::vector new_paths; - std::set_difference(search_results.cbegin(), search_results.cend(), cached_paths.cbegin(), - cached_paths.cend(), std::back_inserter(new_paths)); + std::unordered_set search_results; + search_results.reserve(search_results_vector.size()); + for (const std::string& path : search_results_vector) + { + if (!DiscIO::ShouldHideFromGameList(path)) + search_results.insert(path); + } // Reload the TitleDatabase { @@ -788,27 +781,35 @@ void GameListCtrl::RescanList() m_title_database = {}; } - // For now, only scan new_paths. This could cause false negatives (file actively being written), - // but otherwise should be fine. bool cache_changed = false; { std::unique_lock lk(m_cache_mutex); - for (const auto& path : removed_paths) - { - auto it = std::find_if(m_cached_files.begin(), m_cached_files.end(), - [&path](const std::shared_ptr& file) { - return file->GetFileName() == path; - }); - if (it != m_cached_files.end()) - { - cache_changed = true; - // Efficiently remove the file without caring about preserving any order - *it = std::move(m_cached_files.back()); - m_cached_files.pop_back(); + // Delete paths that aren't in search_results from m_cached_files, + // while simultaneously deleting paths that aren't in m_cached_files from search_results. + // For the sake of speed, we don't care about maintaining the order of m_cached_files. + { + auto it = m_cached_files.begin(); + auto end = m_cached_files.end(); + while (it != end) + { + if (search_results.erase((*it)->GetFileName())) + { + ++it; + } + else + { + cache_changed = true; + --end; + *it = std::move(*end); + } } + m_cached_files.erase(it, m_cached_files.end()); } - for (const auto& path : new_paths) + + // Now that the previous loop has run, search_results only contains paths that + // aren't in m_cached_files, so we simply add all of them to m_cached_files. + for (const auto& path : search_results) { auto file = std::make_shared(path); if (file->IsValid())