Skip to content

Commit 5ee1af3

Browse files
leekilloughKenneth Griesser
authored andcommitted
Use Meyers singletons (sstsimulator#1222)
* 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.
1 parent 2de4955 commit 5ee1af3

File tree

5 files changed

+45
-105
lines changed

5 files changed

+45
-105
lines changed

src/sst/core/elemLoader.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ ElemLoader::loadLibrary(const std::string& elemlib, std::ostream& err_os)
230230
// loop all the elements in the element lib
231231
for ( auto& elempair : libpair.second ) {
232232
// loop all the loaders in the element
233-
for ( auto* loader : elempair.second ) {
233+
for ( auto& loader : elempair.second ) {
234234
loader->load();
235235
}
236236
}

src/sst/core/eli/elementbuilder.h

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,7 @@ class BuilderLibrary
3737

3838
BuilderLibrary(const std::string& name) : name_(name) {}
3939

40-
BaseBuilder* getBuilder(const std::string& name)
41-
{
42-
auto iter = factories_.find(name);
43-
if ( iter == factories_.end() ) { return nullptr; }
44-
else {
45-
return iter->second;
46-
}
47-
}
40+
BaseBuilder* getBuilder(const std::string& name) { return factories_[name]; }
4841

4942
const std::map<std::string, BaseBuilder*>& getMap() const { return factories_; }
5043

@@ -87,29 +80,16 @@ class BuilderLibraryDatabase
8780

8881
static Library* getLibrary(const std::string& name)
8982
{
90-
if ( !libraries ) { libraries = new std::map<std::string, Library*>; }
91-
auto iter = libraries->find(name);
92-
if ( iter == libraries->end() ) {
93-
auto* info = new Library(name);
94-
(*libraries)[name] = info;
95-
return info;
96-
}
97-
else {
98-
return iter->second;
99-
}
83+
static Map libraries; // Database
84+
auto& lib = libraries[name];
85+
if ( !lib ) lib = new Library(name);
86+
return lib;
10087
}
10188

10289
template <class NewBase>
10390
using ChangeBase = BuilderLibraryDatabase<NewBase, CtorArgs...>;
104-
105-
private:
106-
// Database - needs to be a pointer for static init order
107-
static Map* libraries;
10891
};
10992

110-
template <class Base, class... CtorArgs>
111-
typename BuilderLibraryDatabase<Base, CtorArgs...>::Map* BuilderLibraryDatabase<Base, CtorArgs...>::libraries = nullptr;
112-
11393
template <class Base, class Builder, class... CtorArgs>
11494
struct BuilderLoader : public LibraryLoader
11595
{
@@ -146,12 +126,9 @@ struct InstantiateBuilder
146126
{
147127
static bool isLoaded() { return loaded; }
148128

149-
static const bool loaded;
129+
static inline const bool loaded = Base::Ctor::template add<T>();
150130
};
151131

152-
template <class Base, class T>
153-
const bool InstantiateBuilder<Base, T>::loaded = Base::Ctor::template add<T>();
154-
155132
template <class Base, class T, class Enable = void>
156133
struct Allocator
157134
{
@@ -168,14 +145,10 @@ struct CachedAllocator
168145
template <class... Args>
169146
Base* operator()(Args&&... ctorArgs)
170147
{
171-
if ( !cached_ ) { cached_ = new T(std::forward<Args>(ctorArgs)...); }
172-
return cached_;
148+
static T* cached = new T(std::forward<Args>(ctorArgs)...);
149+
return cached;
173150
}
174-
175-
static Base* cached_;
176151
};
177-
template <class Base, class T>
178-
Base* CachedAllocator<Base, T>::cached_ = nullptr;
179152

180153
template <class T, class Base, class... Args>
181154
struct DerivedBuilder : public Builder<Base, Args...>

src/sst/core/eli/elementinfo.h

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -59,32 +59,24 @@ class DataBase
5959
public:
6060
static T* get(const std::string& elemlib, const std::string& elem)
6161
{
62-
if ( !infos_ ) return nullptr;
63-
64-
auto libiter = infos_->find(elemlib);
65-
if ( libiter != infos_->end() ) {
62+
auto libiter = infos().find(elemlib);
63+
if ( libiter != infos().end() ) {
6664
auto& submap = libiter->second;
6765
auto elemiter = submap.find(elem);
6866
if ( elemiter != submap.end() ) { return elemiter->second; }
6967
}
7068
return nullptr;
7169
}
7270

73-
static void add(const std::string& elemlib, const std::string& elem, T* info)
74-
{
75-
if ( !infos_ ) {
76-
infos_ = std::unique_ptr<std::map<std::string, std::map<std::string, T*>>>(
77-
new std::map<std::string, std::map<std::string, T*>>);
78-
}
79-
80-
(*infos_)[elemlib][elem] = info;
81-
}
71+
static void add(const std::string& elemlib, const std::string& elem, T* info) { infos()[elemlib][elem] = info; }
8272

8373
private:
84-
static std::unique_ptr<std::map<std::string, std::map<std::string, T*>>> infos_;
74+
static std::map<std::string, std::map<std::string, T*>>& infos()
75+
{
76+
static std::map<std::string, std::map<std::string, T*>> infos_;
77+
return infos_;
78+
}
8579
};
86-
template <class T>
87-
std::unique_ptr<std::map<std::string, std::map<std::string, T*>>> DataBase<T>::infos_;
8880

8981
template <class Policy, class... Policies>
9082
class BuilderInfoImpl : public Policy, public BuilderInfoImpl<Policies...>
@@ -208,36 +200,30 @@ class InfoLibraryDatabase
208200

209201
std::vector<std::string> ret;
210202
// First iterate over libraries
211-
for ( auto x : (*libraries) ) {
212-
for ( auto& y : x.second->getMap() ) {
213-
ret.push_back(x.first + "." + y.first);
203+
for ( auto& [name, lib] : libraries() ) {
204+
for ( auto& [elemlib, info] : lib->getMap() ) {
205+
ret.push_back(name + "." + elemlib);
214206
}
215207
}
216208
return ret;
217209
}
218210

219211
static Library* getLibrary(const std::string& name)
220212
{
221-
if ( !libraries ) { libraries = new Map; }
222-
auto iter = libraries->find(name);
223-
if ( iter == libraries->end() ) {
224-
auto* info = new Library(name);
225-
(*libraries)[name] = info;
226-
return info;
227-
}
228-
else {
229-
return iter->second;
230-
}
213+
auto& lib = libraries()[name];
214+
if ( !lib ) lib = new Library(name);
215+
return lib;
231216
}
232217

233218
private:
234-
// Database - needs to be a pointer for static init order
235-
static Map* libraries;
219+
// Database
220+
static Map& libraries()
221+
{
222+
static Map libs;
223+
return libs;
224+
}
236225
};
237226

238-
template <class Base>
239-
typename InfoLibraryDatabase<Base>::Map* InfoLibraryDatabase<Base>::libraries = nullptr;
240-
241227
template <class Base, class Info>
242228
struct InfoLoader : public LibraryLoader
243229
{

src/sst/core/eli/elibase.cc

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,16 @@ namespace SST::ELI {
1919
BaseElementInfo class functions
2020
**************************************************************************/
2121

22-
std::unique_ptr<LoadedLibraries::LibraryMap> LoadedLibraries::loaders_ {};
23-
2422
bool
2523
LoadedLibraries::addLoader(
2624
const std::string& lib, const std::string& name, const std::string& alias, LibraryLoader* loader)
2725
{
28-
if ( !loaders_ ) { loaders_ = std::unique_ptr<LibraryMap>(new LibraryMap); }
29-
(*loaders_)[lib][name].push_back(loader);
30-
if ( !alias.empty() ) (*loaders_)[lib][alias].push_back(loader);
31-
return true;
32-
}
26+
std::shared_ptr<LibraryLoader> shared_loader(loader);
3327

34-
const LoadedLibraries::LibraryMap&
35-
LoadedLibraries::getLoaders()
36-
{
37-
if ( !loaders_ ) { loaders_ = std::unique_ptr<LibraryMap>(new LibraryMap); }
38-
return *loaders_;
39-
}
40-
41-
bool
42-
LoadedLibraries::isLoaded(const std::string& name)
43-
{
44-
if ( loaders_ ) { return loaders_->find(name) != loaders_->end(); }
45-
else {
46-
return false; // nothing loaded yet
47-
}
28+
auto& library = getLoaders()[lib];
29+
if ( !alias.empty() && alias != name ) library[alias].push_back(shared_loader);
30+
library[name].push_back(std::move(shared_loader));
31+
return true;
4832
}
4933

5034
} // namespace SST::ELI

src/sst/core/eli/elibase.h

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@
1616

1717
#include <algorithm>
1818
#include <cstring>
19-
#include <functional>
20-
#include <list>
19+
#include <deque>
2120
#include <map>
2221
#include <memory>
23-
#include <set>
2422
#include <string>
2523
#include <type_traits>
2624
#include <vector>
@@ -124,28 +122,27 @@ combineEliInfo(std::vector<T>& base, const std::vector<T>& add)
124122

125123
struct LibraryLoader
126124
{
127-
virtual void load() = 0;
128-
virtual ~LibraryLoader() {}
125+
virtual void load() = 0;
126+
virtual ~LibraryLoader() = default;
129127
};
130128

131129
class LoadedLibraries
132130
{
133131
public:
134-
using InfoMap = std::map<std::string, std::list<LibraryLoader*>>;
132+
using InfoMap = std::map<std::string, std::deque<std::shared_ptr<LibraryLoader>>>;
135133
using LibraryMap = std::map<std::string, InfoMap>;
136134

137-
static bool isLoaded(const std::string& name);
135+
static bool isLoaded(const std::string& name) { return getLoaders().count(name) != 0; }
138136

139-
/**
140-
@return A boolean indicated successfully added
141-
*/
137+
// @return A boolean indicated successfully added
142138
static bool
143139
addLoader(const std::string& lib, const std::string& name, const std::string& alias, LibraryLoader* loader);
144140

145-
static const LibraryMap& getLoaders();
146-
147-
private:
148-
static std::unique_ptr<LibraryMap> loaders_;
141+
static LibraryMap& getLoaders()
142+
{
143+
static LibraryMap loaders;
144+
return loaders;
145+
}
149146
};
150147

151148
// Template used to get aliases. Needed because the ELI_getAlias()

0 commit comments

Comments
 (0)