From 6006b9b48b0c80f4d0e2361cab262d3b0ce18c0c Mon Sep 17 00:00:00 2001 From: leekillough <15950023+leekillough@users.noreply.github.com> Date: Mon, 24 Feb 2025 18:48:00 -0600 Subject: [PATCH 1/4] Use Meyers singletons (static variables local to functions which are initialized on their first call and return a reference to it; avoids static initialization order fiasco) instead of pointers allocated on demand. Use std::shared_ptr for LoadedLibraries loaded libraries so that libraries and aliases can share the same loader pointers without memory leaks. Use C++17 "inline" with static const members so that they can be initialized in-class and won't have multiple definition errors if defined in a header. --- src/sst/core/elemLoader.cc | 2 +- src/sst/core/eli/elementbuilder.h | 43 +++++------------------- src/sst/core/eli/elementinfo.h | 56 ++++++++++++------------------- src/sst/core/eli/elibase.cc | 26 +++----------- src/sst/core/eli/elibase.h | 25 ++++++-------- 5 files changed, 46 insertions(+), 106 deletions(-) diff --git a/src/sst/core/elemLoader.cc b/src/sst/core/elemLoader.cc index f5c3f50b4..8674a5b5e 100644 --- a/src/sst/core/elemLoader.cc +++ b/src/sst/core/elemLoader.cc @@ -230,7 +230,7 @@ ElemLoader::loadLibrary(const std::string& elemlib, std::ostream& err_os) // loop all the elements in the element lib for ( auto& elempair : libpair.second ) { // loop all the loaders in the element - for ( auto* loader : elempair.second ) { + for ( auto& loader : elempair.second ) { loader->load(); } } diff --git a/src/sst/core/eli/elementbuilder.h b/src/sst/core/eli/elementbuilder.h index 29cc97a95..7ca61055f 100644 --- a/src/sst/core/eli/elementbuilder.h +++ b/src/sst/core/eli/elementbuilder.h @@ -38,14 +38,7 @@ class BuilderLibrary BuilderLibrary(const std::string& name) : name_(name) {} - BaseBuilder* getBuilder(const std::string& name) - { - auto iter = factories_.find(name); - if ( iter == factories_.end() ) { return nullptr; } - else { - return iter->second; - } - } + BaseBuilder* getBuilder(const std::string& name) { return factories_[name]; } const std::map& getMap() const { return factories_; } @@ -88,29 +81,16 @@ class BuilderLibraryDatabase static Library* getLibrary(const std::string& name) { - if ( !libraries ) { libraries = new std::map; } - auto iter = libraries->find(name); - if ( iter == libraries->end() ) { - auto* info = new Library(name); - (*libraries)[name] = info; - return info; - } - else { - return iter->second; - } + static Map libraries; // Database + auto& lib = libraries[name]; + if ( !lib ) lib = new Library(name); + return lib; } template using ChangeBase = BuilderLibraryDatabase; - -private: - // Database - needs to be a pointer for static init order - static Map* libraries; }; -template -typename BuilderLibraryDatabase::Map* BuilderLibraryDatabase::libraries = nullptr; - template struct BuilderLoader : public LibraryLoader { @@ -147,12 +127,9 @@ struct InstantiateBuilder { static bool isLoaded() { return loaded; } - static const bool loaded; + static inline const bool loaded = Base::Ctor::template add(); }; -template -const bool InstantiateBuilder::loaded = Base::Ctor::template add(); - template struct Allocator { @@ -169,14 +146,10 @@ struct CachedAllocator template Base* operator()(Args&&... ctorArgs) { - if ( !cached_ ) { cached_ = new T(std::forward(ctorArgs)...); } - return cached_; + static T cached(std::forward(ctorArgs)...); + return &cached; } - - static Base* cached_; }; -template -Base* CachedAllocator::cached_ = nullptr; template struct DerivedBuilder : public Builder diff --git a/src/sst/core/eli/elementinfo.h b/src/sst/core/eli/elementinfo.h index 71d846a71..fe887a175 100644 --- a/src/sst/core/eli/elementinfo.h +++ b/src/sst/core/eli/elementinfo.h @@ -59,10 +59,8 @@ class DataBase public: static T* get(const std::string& elemlib, const std::string& elem) { - if ( !infos_ ) return nullptr; - - auto libiter = infos_->find(elemlib); - if ( libiter != infos_->end() ) { + auto libiter = infos().find(elemlib); + if ( libiter != infos().end() ) { auto& submap = libiter->second; auto elemiter = submap.find(elem); if ( elemiter != submap.end() ) { return elemiter->second; } @@ -70,21 +68,15 @@ class DataBase return nullptr; } - static void add(const std::string& elemlib, const std::string& elem, T* info) - { - if ( !infos_ ) { - infos_ = std::unique_ptr>>( - new std::map>); - } - - (*infos_)[elemlib][elem] = info; - } + static void add(const std::string& elemlib, const std::string& elem, T* info) { infos()[elemlib][elem] = info; } private: - static std::unique_ptr>> infos_; + static auto& infos() + { + static std::map> infos_; + return infos_; + } }; -template -std::unique_ptr>> DataBase::infos_; template class BuilderInfoImpl : public Policy, public BuilderInfoImpl @@ -166,7 +158,7 @@ class InfoLibrary return count; } - const std::map& getMap() const { return infos_; } + const auto& getMap() const { return infos_; } void readdInfo(const std::string& name, BaseInfo* info) { @@ -208,9 +200,9 @@ class InfoLibraryDatabase std::vector ret; // First iterate over libraries - for ( auto x : (*libraries) ) { - for ( auto& y : x.second->getMap() ) { - ret.push_back(x.first + "." + y.first); + for ( auto& [name, lib] : libraries() ) { + for ( auto& [elemlib, info] : lib->getMap() ) { + ret.push_back(name + "." + elemlib); } } return ret; @@ -218,26 +210,20 @@ class InfoLibraryDatabase static Library* getLibrary(const std::string& name) { - if ( !libraries ) { libraries = new Map; } - auto iter = libraries->find(name); - if ( iter == libraries->end() ) { - auto* info = new Library(name); - (*libraries)[name] = info; - return info; - } - else { - return iter->second; - } + auto& lib = libraries()[name]; + if ( !lib ) lib = new Library(name); + return lib; } private: - // Database - needs to be a pointer for static init order - static Map* libraries; + // Database + static Map& libraries() + { + static Map libs; + return libs; + } }; -template -typename InfoLibraryDatabase::Map* InfoLibraryDatabase::libraries = nullptr; - template struct InfoLoader : public LibraryLoader { diff --git a/src/sst/core/eli/elibase.cc b/src/sst/core/eli/elibase.cc index 0bd58c005..d38df1365 100644 --- a/src/sst/core/eli/elibase.cc +++ b/src/sst/core/eli/elibase.cc @@ -20,32 +20,16 @@ namespace SST { **************************************************************************/ namespace ELI { -std::unique_ptr LoadedLibraries::loaders_ {}; - bool LoadedLibraries::addLoader( const std::string& lib, const std::string& name, const std::string& alias, LibraryLoader* loader) { - if ( !loaders_ ) { loaders_ = std::unique_ptr(new LibraryMap); } - (*loaders_)[lib][name].push_back(loader); - if ( !alias.empty() ) (*loaders_)[lib][alias].push_back(loader); - return true; -} + std::shared_ptr shared_loader(loader); -const LoadedLibraries::LibraryMap& -LoadedLibraries::getLoaders() -{ - if ( !loaders_ ) { loaders_ = std::unique_ptr(new LibraryMap); } - return *loaders_; -} - -bool -LoadedLibraries::isLoaded(const std::string& name) -{ - if ( loaders_ ) { return loaders_->find(name) != loaders_->end(); } - else { - return false; // nothing loaded yet - } + auto& library = getLoaders()[lib]; + if ( !alias.empty() && alias != name ) library[alias].push_back(shared_loader); + library[name].push_back(std::move(shared_loader)); + return true; } } // namespace ELI diff --git a/src/sst/core/eli/elibase.h b/src/sst/core/eli/elibase.h index 717045755..4ea757b31 100644 --- a/src/sst/core/eli/elibase.h +++ b/src/sst/core/eli/elibase.h @@ -16,11 +16,9 @@ #include #include -#include -#include +#include #include #include -#include #include #include #include @@ -124,28 +122,27 @@ combineEliInfo(std::vector& base, const std::vector& add) struct LibraryLoader { - virtual void load() = 0; - virtual ~LibraryLoader() {} + virtual void load() = 0; + virtual ~LibraryLoader() = default; }; class LoadedLibraries { public: - using InfoMap = std::map>; + using InfoMap = std::map>>; using LibraryMap = std::map; - static bool isLoaded(const std::string& name); + static bool isLoaded(const std::string& name) { return getLoaders().count(name) != 0; } - /** - @return A boolean indicated successfully added - */ + // @return A boolean indicated successfully added static bool addLoader(const std::string& lib, const std::string& name, const std::string& alias, LibraryLoader* loader); - static const LibraryMap& getLoaders(); - -private: - static std::unique_ptr loaders_; + static LibraryMap& getLoaders() + { + static LibraryMap loaders; + return loaders; + } }; // Template used to get aliases. Needed because the ELI_getAlias() From 6cc661259c2e59747bc78eb349f9833ccf86ddf9 Mon Sep 17 00:00:00 2001 From: leekillough <15950023+leekillough@users.noreply.github.com> Date: Tue, 4 Mar 2025 13:51:18 -0600 Subject: [PATCH 2/4] fix merge --- src/sst/core/eli/elibase.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sst/core/eli/elibase.cc b/src/sst/core/eli/elibase.cc index 4e2df0b3f..892bce120 100644 --- a/src/sst/core/eli/elibase.cc +++ b/src/sst/core/eli/elibase.cc @@ -30,3 +30,5 @@ LoadedLibraries::addLoader( library[name].push_back(std::move(shared_loader)); return true; } + +} // namespace SST::ELI From 11bd11c11df2a440fef872d5eb0c7592c1a20fbc Mon Sep 17 00:00:00 2001 From: leekillough <15950023+leekillough@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:10:40 -0600 Subject: [PATCH 3/4] don't use auto for functions --- src/sst/core/eli/elementinfo.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sst/core/eli/elementinfo.h b/src/sst/core/eli/elementinfo.h index fe887a175..5d087f48a 100644 --- a/src/sst/core/eli/elementinfo.h +++ b/src/sst/core/eli/elementinfo.h @@ -71,7 +71,7 @@ class DataBase static void add(const std::string& elemlib, const std::string& elem, T* info) { infos()[elemlib][elem] = info; } private: - static auto& infos() + static std::map>& infos() { static std::map> infos_; return infos_; @@ -158,7 +158,7 @@ class InfoLibrary return count; } - const auto& getMap() const { return infos_; } + const std::map& getMap() const { return infos_; } void readdInfo(const std::string& name, BaseInfo* info) { From a09dc3706f881c19d955519c7e981ae5f5dcbc44 Mon Sep 17 00:00:00 2001 From: leekillough <15950023+leekillough@users.noreply.github.com> Date: Wed, 5 Mar 2025 21:20:14 -0600 Subject: [PATCH 4/4] make CachedAllocator() use allocated memory --- src/sst/core/eli/elementbuilder.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sst/core/eli/elementbuilder.h b/src/sst/core/eli/elementbuilder.h index 4ab34ff36..76887c871 100644 --- a/src/sst/core/eli/elementbuilder.h +++ b/src/sst/core/eli/elementbuilder.h @@ -145,8 +145,8 @@ struct CachedAllocator template Base* operator()(Args&&... ctorArgs) { - static T cached(std::forward(ctorArgs)...); - return &cached; + static T* cached = new T(std::forward(ctorArgs)...); + return cached; } };