From f2188d25336a0721d874a9d1436dc2eb3ad60c6e Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 15 May 2021 02:29:50 +0200 Subject: [PATCH 1/4] Reduce temporary allocations on ESM loading By moving objects instead of copying when possible. --- apps/opencs/model/world/refcollection.cpp | 8 +++++--- apps/openmw/mwrender/groundcover.cpp | 2 +- apps/openmw/mwrender/objectpaging.cpp | 4 ++-- apps/openmw/mwworld/cellstore.cpp | 3 ++- apps/openmw/mwworld/esmstore.cpp | 12 ++++++------ apps/openmw/mwworld/store.cpp | 4 ++-- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/apps/opencs/model/world/refcollection.cpp b/apps/opencs/model/world/refcollection.cpp index 51588cbc2..dfdb8e73b 100644 --- a/apps/opencs/model/world/refcollection.cpp +++ b/apps/opencs/model/world/refcollection.cpp @@ -108,11 +108,13 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool Record record; record.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_ModifiedOnly; - (base ? record.mBase : record.mModified) = ref; + const ESM::RefNum refNum = ref.mRefNum; + std::string refId = ref.mId; + (base ? record.mBase : record.mModified) = std::move(ref); appendRecord (record); - cache.insert (std::make_pair (ref.mRefNum, ref.mId)); + cache.emplace(refNum, std::move(refId)); } else { @@ -123,7 +125,7 @@ void CSMWorld::RefCollection::load (ESM::ESMReader& reader, int cellIndex, bool Record record = getRecord (index); record.mState = base ? RecordBase::State_BaseOnly : RecordBase::State_Modified; - (base ? record.mBase : record.mModified) = ref; + (base ? record.mBase : record.mModified) = std::move(ref); setRecord (index, record); } diff --git a/apps/openmw/mwrender/groundcover.cpp b/apps/openmw/mwrender/groundcover.cpp index fd2246253..63df3e249 100644 --- a/apps/openmw/mwrender/groundcover.cpp +++ b/apps/openmw/mwrender/groundcover.cpp @@ -241,7 +241,7 @@ namespace MWRender if (model.empty()) continue; model = "meshes/" + model; - instances[model].emplace_back(ref, model); + instances[model].emplace_back(std::move(ref), std::move(model)); } } } diff --git a/apps/openmw/mwrender/objectpaging.cpp b/apps/openmw/mwrender/objectpaging.cpp index 4217c4714..05acb35d1 100644 --- a/apps/openmw/mwrender/objectpaging.cpp +++ b/apps/openmw/mwrender/objectpaging.cpp @@ -432,7 +432,7 @@ namespace MWRender if (!typeFilter(type,size>=2)) continue; if (deleted) { refs.erase(ref.mRefNum); continue; } if (ref.mRefNum.fromGroundcoverFile()) continue; - refs[ref.mRefNum] = ref; + refs[ref.mRefNum] = std::move(ref); } } catch (std::exception&) @@ -448,7 +448,7 @@ namespace MWRender if (deleted) { refs.erase(ref.mRefNum); continue; } int type = store.findStatic(ref.mRefID); if (!typeFilter(type,size>=2)) continue; - refs[ref.mRefNum] = ref; + refs[ref.mRefNum] = std::move(ref); } } } diff --git a/apps/openmw/mwworld/cellstore.cpp b/apps/openmw/mwworld/cellstore.cpp index 73f69e220..813042f15 100644 --- a/apps/openmw/mwworld/cellstore.cpp +++ b/apps/openmw/mwworld/cellstore.cpp @@ -566,7 +566,8 @@ namespace MWWorld continue; } - mIds.push_back (Misc::StringUtils::lowerCase (ref.mRefID)); + Misc::StringUtils::lowerCaseInPlace(ref.mRefID); + mIds.push_back(std::move(ref.mRefID)); } } catch (std::exception& e) diff --git a/apps/openmw/mwworld/esmstore.cpp b/apps/openmw/mwworld/esmstore.cpp index a69d74b05..8b2eeb2fc 100644 --- a/apps/openmw/mwworld/esmstore.cpp +++ b/apps/openmw/mwworld/esmstore.cpp @@ -31,7 +31,7 @@ namespace else if (std::find(cell.mMovedRefs.begin(), cell.mMovedRefs.end(), ref.mRefNum) == cell.mMovedRefs.end()) { Misc::StringUtils::lowerCaseInPlace(ref.mRefID); - refs[ref.mRefNum] = ref.mRefID; + refs[ref.mRefNum] = std::move(ref.mRefID); } } } @@ -42,9 +42,9 @@ namespace refs.erase(it.first.mRefNum); else { - ESM::CellRef ref = it.first; - Misc::StringUtils::lowerCaseInPlace(ref.mRefID); - refs[ref.mRefNum] = ref.mRefID; + std::string refId = it.first.mRefID; + Misc::StringUtils::lowerCaseInPlace(refId); + refs[it.first.mRefNum] = std::move(refId); } } } @@ -254,8 +254,8 @@ void ESMStore::countRecords() readRefs(*it, refs, readers); for(auto it = mCells.extBegin(); it != mCells.extEnd(); it++) readRefs(*it, refs, readers); - for(const auto& pair : refs) - mRefCount[pair.second]++; + for(auto& pair : refs) + mRefCount[std::move(pair.second)]++; } int ESMStore::getRefCount(const std::string& id) const diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 53ba3cfc2..8e6596cd3 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -518,9 +518,9 @@ namespace MWWorld // But there may be duplicates here! ESM::CellRefTracker::iterator iter = std::find_if(cellAlt->mLeasedRefs.begin(), cellAlt->mLeasedRefs.end(), ESM::CellRefTrackerPredicate(ref.mRefNum)); if (iter == cellAlt->mLeasedRefs.end()) - cellAlt->mLeasedRefs.push_back(std::make_pair(ref, deleted)); + cellAlt->mLeasedRefs.emplace_back(std::move(ref), deleted); else - *iter = std::make_pair(ref, deleted); + *iter = std::make_pair(std::move(ref), deleted); } } const ESM::Cell *Store::search(const std::string &id) const From 6248dc72cbaf079608619bf195783e42d3693b13 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 15 May 2021 03:43:26 +0200 Subject: [PATCH 2/4] Convert to lower case only when needed --- apps/openmw/mwrender/objectpaging.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/openmw/mwrender/objectpaging.cpp b/apps/openmw/mwrender/objectpaging.cpp index 05acb35d1..72ea70000 100644 --- a/apps/openmw/mwrender/objectpaging.cpp +++ b/apps/openmw/mwrender/objectpaging.cpp @@ -426,8 +426,8 @@ namespace MWRender bool deleted = false; while(cell->getNextRef(esm[index], ref, deleted)) { - Misc::StringUtils::lowerCaseInPlace(ref.mRefID); if (std::find(cell->mMovedRefs.begin(), cell->mMovedRefs.end(), ref.mRefNum) != cell->mMovedRefs.end()) continue; + Misc::StringUtils::lowerCaseInPlace(ref.mRefID); int type = store.findStatic(ref.mRefID); if (!typeFilter(type,size>=2)) continue; if (deleted) { refs.erase(ref.mRefNum); continue; } @@ -443,9 +443,9 @@ namespace MWRender for (ESM::CellRefTracker::const_iterator it = cell->mLeasedRefs.begin(); it != cell->mLeasedRefs.end(); ++it) { ESM::CellRef ref = it->first; - Misc::StringUtils::lowerCaseInPlace(ref.mRefID); bool deleted = it->second; if (deleted) { refs.erase(ref.mRefNum); continue; } + Misc::StringUtils::lowerCaseInPlace(ref.mRefID); int type = store.findStatic(ref.mRefID); if (!typeFilter(type,size>=2)) continue; refs[ref.mRefNum] = std::move(ref); From 9938af2289d3cae5b4c476ea75c0c48a3b517b81 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 6 Apr 2021 23:58:57 +0200 Subject: [PATCH 3/4] Use unordered_map for ref count Reduces ESMStore::countRecords time by 8%. --- apps/openmw/mwworld/esmstore.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/openmw/mwworld/esmstore.hpp b/apps/openmw/mwworld/esmstore.hpp index 26f497a52..608b5489e 100644 --- a/apps/openmw/mwworld/esmstore.hpp +++ b/apps/openmw/mwworld/esmstore.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "store.hpp" @@ -76,7 +77,7 @@ namespace MWWorld std::map mIds; std::map mStaticIds; - std::map mRefCount; + std::unordered_map mRefCount; std::map mStores; From 1e2aae809516d7051c36f94863f51f603de38547 Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 6 Apr 2021 23:59:58 +0200 Subject: [PATCH 4/4] Use stable sort+unique to collect RefIDs for ESMStore records counting The idea is to avoid std::map lookup for each CellRef. Instead generate a sequence of added and removed RefNums into a vector then order them by RefNum using a stable sort that preserves relative order of elements with the same RefNum. RefIDs are stored in a different vector to avoid std::string move ctor calls when swapping elements while sorting. Reversed iteration over added and removed RefNums for each unique RefNum is an equivalent to what map-based algorithm produces. The main benefit from sorting a vector is a data locality that means less cache misses for each access. Reduces ESMStore::countRecords perf cycles by 25%. --- apps/openmw/mwworld/esmstore.cpp | 52 +++++++++++++++++++++++--------- components/misc/algorithm.hpp | 36 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 components/misc/algorithm.hpp diff --git a/apps/openmw/mwworld/esmstore.cpp b/apps/openmw/mwworld/esmstore.cpp index 8b2eeb2fc..71ee2f5c3 100644 --- a/apps/openmw/mwworld/esmstore.cpp +++ b/apps/openmw/mwworld/esmstore.cpp @@ -1,5 +1,6 @@ #include "esmstore.hpp" +#include #include #include @@ -8,12 +9,23 @@ #include #include #include +#include #include "../mwmechanics/spelllist.hpp" namespace { - void readRefs(const ESM::Cell& cell, std::map& refs, std::vector& readers) + struct Ref + { + ESM::RefNum mRefNum; + std::size_t mRefID; + + Ref(ESM::RefNum refNum, std::size_t refID) : mRefNum(refNum), mRefID(refID) {} + }; + + constexpr std::size_t deletedRefID = std::numeric_limits::max(); + + void readRefs(const ESM::Cell& cell, std::vector& refs, std::vector& refIDs, std::vector& readers) { for (size_t i = 0; i < cell.mContextList.size(); i++) { @@ -27,24 +39,22 @@ namespace while(cell.getNextRef(readers[index], ref, deleted)) { if(deleted) - refs.erase(ref.mRefNum); + refs.emplace_back(ref.mRefNum, deletedRefID); else if (std::find(cell.mMovedRefs.begin(), cell.mMovedRefs.end(), ref.mRefNum) == cell.mMovedRefs.end()) { - Misc::StringUtils::lowerCaseInPlace(ref.mRefID); - refs[ref.mRefNum] = std::move(ref.mRefID); + refs.emplace_back(ref.mRefNum, refIDs.size()); + refIDs.push_back(std::move(ref.mRefID)); } } } - for(const auto& it : cell.mLeasedRefs) + for(const auto& [value, deleted] : cell.mLeasedRefs) { - bool deleted = it.second; if(deleted) - refs.erase(it.first.mRefNum); + refs.emplace_back(value.mRefNum, deletedRefID); else { - std::string refId = it.first.mRefID; - Misc::StringUtils::lowerCaseInPlace(refId); - refs[it.first.mRefNum] = std::move(refId); + refs.emplace_back(value.mRefNum, refIDs.size()); + refIDs.push_back(value.mRefID); } } } @@ -248,14 +258,26 @@ void ESMStore::countRecords() { if(!mRefCount.empty()) return; - std::map refs; + std::vector refs; + std::vector refIDs; std::vector readers; for(auto it = mCells.intBegin(); it != mCells.intEnd(); it++) - readRefs(*it, refs, readers); + readRefs(*it, refs, refIDs, readers); for(auto it = mCells.extBegin(); it != mCells.extEnd(); it++) - readRefs(*it, refs, readers); - for(auto& pair : refs) - mRefCount[std::move(pair.second)]++; + readRefs(*it, refs, refIDs, readers); + const auto lessByRefNum = [] (const Ref& l, const Ref& r) { return l.mRefNum < r.mRefNum; }; + std::stable_sort(refs.begin(), refs.end(), lessByRefNum); + const auto equalByRefNum = [] (const Ref& l, const Ref& r) { return l.mRefNum == r.mRefNum; }; + const auto incrementRefCount = [&] (const Ref& value) + { + if (value.mRefID != deletedRefID) + { + std::string& refId = refIDs[value.mRefID]; + Misc::StringUtils::lowerCaseInPlace(refId); + ++mRefCount[std::move(refId)]; + } + }; + Misc::forEachUnique(refs.rbegin(), refs.rend(), equalByRefNum, incrementRefCount); } int ESMStore::getRefCount(const std::string& id) const diff --git a/components/misc/algorithm.hpp b/components/misc/algorithm.hpp new file mode 100644 index 000000000..4d70afa86 --- /dev/null +++ b/components/misc/algorithm.hpp @@ -0,0 +1,36 @@ +#ifndef OPENMW_COMPONENTS_MISC_ALGORITHM_H +#define OPENMW_COMPONENTS_MISC_ALGORITHM_H + +#include +#include + +namespace Misc +{ + template + inline Iterator forEachUnique(Iterator begin, Iterator end, BinaryPredicate predicate, Function function) + { + static_assert( + std::is_base_of_v< + std::forward_iterator_tag, + typename std::iterator_traits::iterator_category + > + ); + if (begin == end) + return begin; + function(*begin); + auto last = begin; + ++begin; + while (begin != end) + { + if (!predicate(*begin, *last)) + { + function(*begin); + last = begin; + } + ++begin; + } + return begin; + } +} + +#endif