Skip to content

Commit

Permalink
Don't generate a mutex for each Locked<T>, share one per object
Browse files Browse the repository at this point in the history
This reduces RAM usage per object by sizeof(mutex)*(FIELDS-1).
  • Loading branch information
Al2Klimov committed Sep 26, 2024
1 parent 01d3a1d commit d33f5ef
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 30 deletions.
75 changes: 60 additions & 15 deletions lib/base/atomic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class Atomic : public std::atomic<T> {
}
};

class LockedMutex;

/**
* Wraps any T into a std::atomic<T>-like interface that locks using a mutex.
*
Expand All @@ -53,37 +55,80 @@ template<typename T>
class Locked
{
public:
inline T load() const
{
std::unique_lock<std::mutex> lock(m_Mutex);
T load(LockedMutex& mtx) const;
void store(T desired, LockedMutex& mtx);

return m_Value;
}
private:
T m_Value;
};

inline void store(T desired)
{
std::unique_lock<std::mutex> lock(m_Mutex);
/**
* Wraps std::mutex, so that only Locked<T> can (un)lock it.
*
* The latter tiny lock scope is enforced this way to prevent deadlocks while passing around mutexes.
*
* @ingroup base
*/
class LockedMutex
{
template<class T>
friend class Locked;

private:
std::mutex m_Mutex;
};

template<class T>
T Locked<T>::load(LockedMutex& mtx) const
{
std::unique_lock lock (mtx.m_Mutex);

m_Value = std::move(desired);
return m_Value;
}

template<class T>
void Locked<T>::store(T desired, LockedMutex& mtx)
{
std::unique_lock lock (mtx.m_Mutex);

m_Value = std::move(desired);
}

/**
* Extends std::atomic with a Locked-like interface.
*
* @ingroup base
*/
template<class T>
class AtomicPseudoLocked : public std::atomic<T> {
public:
using std::atomic<T>::atomic;
using std::atomic<T>::load;
using std::atomic<T>::store;

T load(LockedMutex&) const
{
return load();
}

private:
mutable std::mutex m_Mutex;
T m_Value;
void store(T desired, LockedMutex&)
{
store(desired);
}
};

/**
* Type alias for std::atomic<T> if possible, otherwise Locked<T> is used as a fallback.
* Type alias for AtomicPseudoLocked<T> if possible, otherwise Locked<T> is used as a fallback.
*
* @ingroup base
*/
template <typename T>
using AtomicOrLocked =
#if defined(__GNUC__) && __GNUC__ < 5
// GCC does not implement std::is_trivially_copyable until version 5.
typename std::conditional<std::is_fundamental<T>::value || std::is_pointer<T>::value, std::atomic<T>, Locked<T>>::type;
typename std::conditional<std::is_fundamental<T>::value || std::is_pointer<T>::value, AtomicPseudoLocked<T>, Locked<T>>::type;
#else /* defined(__GNUC__) && __GNUC__ < 5 */
typename std::conditional<std::is_trivially_copyable<T>::value, std::atomic<T>, Locked<T>>::type;
typename std::conditional<std::is_trivially_copyable<T>::value, AtomicPseudoLocked<T>, Locked<T>>::type;
#endif /* defined(__GNUC__) && __GNUC__ < 5 */

}
Expand Down
2 changes: 1 addition & 1 deletion lib/base/configobject.ti
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ abstract class ConfigObject : ConfigObjectBase < ConfigType
[config, no_user_modify] String __name (Name);
[config, no_user_modify, required] String "name" (ShortName) {
get {{{
String shortName = m_ShortName.load();
String shortName = m_ShortName.load(m_FieldsMutex);
if (shortName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/host.ti
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Host : Checkable

[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/hostgroup.ti
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class HostGroup : CustomVarObject
{
[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/service.ti
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Service : Checkable < ServiceNameComposer

[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetShortName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/servicegroup.ti
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class ServiceGroup : CustomVarObject
{
[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/timeperiod.ti
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class TimePeriod : CustomVarObject
{
[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/user.ti
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class User : CustomVarObject
{
[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
2 changes: 1 addition & 1 deletion lib/icinga/usergroup.ti
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class UserGroup : CustomVarObject
{
[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
String displayName = m_DisplayName.load(m_FieldsMutex);
if (displayName.IsEmpty())
return GetName();
else
Expand Down
4 changes: 2 additions & 2 deletions lib/icingadb/icingadb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ REGISTER_TYPE(IcingaDB);
IcingaDB::IcingaDB()
: m_Rcon(nullptr)
{
m_RconLocked.store(nullptr);
m_RconLocked.store(nullptr, m_FieldsMutex);

m_WorkQueue.SetName("IcingaDB");

Expand Down Expand Up @@ -80,7 +80,7 @@ void IcingaDB::Start(bool runtimeCreated)
m_Rcon = new RedisConnection(GetHost(), GetPort(), GetPath(), GetPassword(), GetDbIndex(),
GetEnableTls(), GetInsecureNoverify(), GetCertPath(), GetKeyPath(), GetCaPath(), GetCrlPath(),
GetTlsProtocolmin(), GetCipherList(), GetConnectTimeout(), GetDebugInfo());
m_RconLocked.store(m_Rcon);
m_RconLocked.store(m_Rcon, m_FieldsMutex);

for (const Type::Ptr& type : GetTypes()) {
auto ctype (dynamic_cast<ConfigType*>(type.get()));
Expand Down
2 changes: 1 addition & 1 deletion lib/icingadb/icingadb.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class IcingaDB : public ObjectImpl<IcingaDB>

inline RedisConnection::Ptr GetConnection()
{
return m_RconLocked.load();
return m_RconLocked.load(m_FieldsMutex);
}

template<class T>
Expand Down
14 changes: 10 additions & 4 deletions tools/mkclass/classcompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,14 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
m_Header << "template<>" << std::endl
<< "class ObjectImpl<" << klass.Name << ">"
<< " : public " << (klass.Parent.empty() ? "Object" : klass.Parent) << std::endl
<< "{" << std::endl
<< "public:" << std::endl
<< "{" << std::endl;

if (klass.Parent.empty()) {
m_Header << "protected:" << std::endl
<< "\tmutable LockedMutex m_FieldsMutex;" << std::endl << std::endl;
}

m_Header << "public:" << std::endl
<< "\t" << "DECLARE_PTR_TYPEDEFS(ObjectImpl<" << klass.Name << ">);" << std::endl << std::endl;

/* Validate */
Expand Down Expand Up @@ -815,7 +821,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
<< "{" << std::endl;

if (field.GetAccessor.empty() && !(field.Attributes & FANoStorage))
m_Impl << "\t" << "return m_" << field.GetFriendlyName() << ".load();" << std::endl;
m_Impl << "\t" << "return m_" << field.GetFriendlyName() << ".load(m_FieldsMutex);" << std::endl;
else
m_Impl << field.GetAccessor << std::endl;

Expand Down Expand Up @@ -853,7 +859,7 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&)
<< "\t" << "auto *dobj = dynamic_cast<ConfigObject *>(this);" << std::endl;

if (field.SetAccessor.empty() && !(field.Attributes & FANoStorage))
m_Impl << "\t" << "m_" << field.GetFriendlyName() << ".store(value);" << std::endl;
m_Impl << "\t" << "m_" << field.GetFriendlyName() << ".store(value, m_FieldsMutex);" << std::endl;
else
m_Impl << field.SetAccessor << std::endl << std::endl;

Expand Down

0 comments on commit d33f5ef

Please sign in to comment.