From 8c40db89fd079efc471b97131c9758df807624e5 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 19 Dec 2022 16:31:12 +0000 Subject: [PATCH] Use C++17 parallel algorithms during plugin loading This requires linking to the TBB library on Linux, where libstdc++ and libc++ don't provide an implementation. --- .github/workflows/ci.yml | 1 + .github/workflows/release.yml | 1 + CMakeLists.txt | 3 +- docs/api/sorting.rst | 12 +-- src/api/game/game.cpp | 76 ++++--------------- src/tests/api/interface/game_interface_test.h | 10 +++ 6 files changed, 33 insertions(+), 70 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df93a7ee..ddded07e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,6 +100,7 @@ jobs: language-pack-el \ language-pack-tr \ libicu-dev \ + libtbb-dev \ g++-10 - name: Run CMake diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index eb7d4a8d..82087104 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -67,6 +67,7 @@ jobs: language-pack-el \ language-pack-tr \ libicu-dev \ + libtbb-dev \ g++-10 - name: Run CMake diff --git a/CMakeLists.txt b/CMakeLists.txt index fb30298b..0fa9ba6e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -54,6 +54,7 @@ find_package(Boost REQUIRED) if(NOT CMAKE_SYSTEM_NAME STREQUAL "Windows") find_package(ICU REQUIRED COMPONENTS uc) + find_package(TBB REQUIRED) endif() ExternalProject_Add(GTest @@ -428,7 +429,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows") target_link_libraries(libloot_internals_tests PRIVATE ${LOOT_LIBS}) target_link_libraries(loot PRIVATE ${LOOT_LIBS}) else() - set(LOOT_LIBS ICU::uc pthread stdc++fs) + set(LOOT_LIBS ICU::uc pthread stdc++fs TBB::tbb) target_link_libraries(libloot_internals_tests PRIVATE ${LOOT_LIBS}) target_link_libraries(loot PRIVATE ${LOOT_LIBS}) diff --git a/docs/api/sorting.rst b/docs/api/sorting.rst index b25c8823..79a39b2d 100644 --- a/docs/api/sorting.rst +++ b/docs/api/sorting.rst @@ -11,15 +11,9 @@ Load plugin data ================ In this first stage, the plugins to be sorted are parsed and their record IDs -(which are FormIDs for all games apart from Morrowind) are stored. Parsing is -multithreaded by dividing the plugins into buckets with roughly equal total file -sizes, and loading each bucket's plugins in a separate thread. The number of -buckets created is equal to the number of concurrent threads that are -hardware-supported (e.g. a dual-core CPU without hyperthreading may report that -it supports two threads). - -When parsing plugins, all subrecords are skipped over for efficiency, apart from -the subrecords of the ``TES4`` header record. +(which are FormIDs for all games apart from Morrowind) are stored. When parsing +plugins, all subrecords are skipped over for efficiency, apart from the +subrecords of the ``TES4`` header record. Loading plugin data also involves loading any metadata that the plugin may have in the masterlist and userlist. diff --git a/src/api/game/game.cpp b/src/api/game/game.cpp index a4101bca..d52a4fa2 100644 --- a/src/api/game/game.cpp +++ b/src/api/game/game.cpp @@ -27,7 +27,7 @@ #include #include #include -#include +#include #include #include "api/api_database.h" @@ -91,53 +91,11 @@ bool Game::IsValidPlugin(const std::string& plugin) const { void Game::LoadPlugins(const std::vector& plugins, bool loadHeadersOnly) { - auto logger = getLogger(); - std::multimap sizeMap; + const auto logger = getLogger(); - // First get the plugin sizes. for (const auto& plugin : plugins) { if (!IsValidPlugin(plugin)) throw std::invalid_argument("\"" + plugin + "\" is not a valid plugin"); - - uintmax_t fileSize = Plugin::GetFileSize(DataPath() / u8path(plugin)); - - // Trim .ghost extension if present. - if (boost::iends_with(plugin, GHOST_FILE_EXTENSION)) - sizeMap.emplace( - fileSize, - plugin.substr(0, plugin.length() - GHOST_FILE_EXTENSION_LENGTH)); - else - sizeMap.emplace(fileSize, plugin); - } - - // Get the number of threads to use. - // hardware_concurrency() may be zero, if so then use only one thread. - size_t threadsToUse = - ::std::min((size_t)std::thread::hardware_concurrency(), sizeMap.size()); - threadsToUse = ::std::max(threadsToUse, (size_t)1); - - // Divide the plugins up by thread. - std::vector> pluginGroups(threadsToUse); - if (logger) { - auto pluginsPerThread = sizeMap.size() / threadsToUse; - logger->info( - "Loading {} plugins using {} threads, with up to {} plugins per " - "thread.", - sizeMap.size(), - threadsToUse, - pluginsPerThread); - } - - // The plugins should be split between the threads so that the data - // load is as evenly spread as possible. - size_t currentGroup = 0; - for (const auto& plugin : sizeMap) { - if (currentGroup == threadsToUse) { - currentGroup = 0; - } - - pluginGroups.at(currentGroup).push_back(plugin.second); - ++currentGroup; } // Clear the existing plugin and archive caches. @@ -150,14 +108,20 @@ void Game::LoadPlugins(const std::vector& plugins, if (logger) { logger->trace("Starting plugin loading."); } - auto masterPath = DataPath() / u8path(masterFilename_); - std::vector threads; - while (threads.size() < threadsToUse) { - const auto& pluginGroup = pluginGroups.at(threads.size()); - threads.push_back(std::thread([&]() { - for (auto pluginName : pluginGroup) { + + const auto masterPath = DataPath() / u8path(masterFilename_); + std::for_each( + std::execution::par_unseq, + plugins.begin(), + plugins.end(), + [&](const std::string& pluginName) { try { - auto pluginPath = DataPath() / u8path(pluginName); + const auto endIt = + boost::iends_with(pluginName, GHOST_FILE_EXTENSION) + ? pluginName.end() - GHOST_FILE_EXTENSION_LENGTH + : pluginName.end(); + + auto pluginPath = DataPath() / u8path(pluginName.begin(), endIt); const bool loadHeader = loadHeadersOnly || loot::equivalent(pluginPath, masterPath); @@ -170,15 +134,7 @@ void Game::LoadPlugins(const std::vector& plugins, e.what()); } } - } - })); - } - - // Join all threads. - for (auto& thread : threads) { - if (thread.joinable()) - thread.join(); - } + }); conditionEvaluator_->RefreshLoadedPluginsState(GetLoadedPlugins()); } diff --git a/src/tests/api/interface/game_interface_test.h b/src/tests/api/interface/game_interface_test.h index 9f3b891a..335c8f7c 100644 --- a/src/tests/api/interface/game_interface_test.h +++ b/src/tests/api/interface/game_interface_test.h @@ -106,6 +106,16 @@ TEST_P( EXPECT_FALSE(plugin->GetCRC()); } +TEST_P(GameInterfaceTest, loadPluginsShouldTrimDotGhostFileExtensions) { + handle_->LoadPlugins({blankMasterDependentEsm + ".ghost"}, true); + EXPECT_EQ(1, handle_->GetLoadedPlugins().size()); + + ASSERT_NO_THROW(handle_->GetPlugin(blankMasterDependentEsm)); + const auto plugin = handle_->GetPlugin(blankMasterDependentEsm); + ASSERT_NE(nullptr, plugin); + EXPECT_EQ(blankMasterDependentEsm, plugin->GetName()); +} + TEST_P(GameInterfaceTest, loadPluginsWithHeadersOnlyFalseShouldFullyLoadAllInstalledPlugins) { handle_->LoadPlugins(pluginsToLoad, false);