Don't generate a mutex for each Locked<T>, share one per object

This reduces RAM usage per object by sizeof(mutex)*(FIELDS-1).
Icinga · Aug 20, 2024 · 250635f · 250635f
1 parent 0463607
commit 250635f
Show file tree

Hide file tree

Showing 12 changed files with 51 additions and 23 deletions.
diff --git a/lib/base/atomic.hpp b/lib/base/atomic.hpp
@@ -53,37 +53,59 @@ template<typename T>
 class Locked
 {
 public:
-	inline T load() const
+	T load(std::mutex& mtx) const
 	{
-		std::unique_lock<std::mutex> lock(m_Mutex);
+		std::unique_lock lock (mtx);
 
 		return m_Value;
 	}
 
-	inline void store(T desired)
+	void store(T desired, std::mutex& mtx)
 	{
-		std::unique_lock<std::mutex> lock(m_Mutex);
+		std::unique_lock lock (mtx);
 
 		m_Value = std::move(desired);
 	}
 
 private:
-	mutable std::mutex m_Mutex;
 	T m_Value;
 };
 
 /**
- * Type alias for std::atomic<T> if possible, otherwise Locked<T> is used as a fallback.
+ * 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(std::mutex&) const
+	{
+		return load();
+	}
+
+	void store(T desired, std::mutex&)
+	{
+		store(desired);
+	}
+};
+
+/**
+ * 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 */
 
 }

diff --git a/lib/base/configobject.ti b/lib/base/configobject.ti
@@ -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

diff --git a/lib/icinga/host.ti b/lib/icinga/host.ti
@@ -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

diff --git a/lib/icinga/hostgroup.ti b/lib/icinga/hostgroup.ti
@@ -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

diff --git a/lib/icinga/service.ti b/lib/icinga/service.ti
@@ -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

diff --git a/lib/icinga/servicegroup.ti b/lib/icinga/servicegroup.ti
@@ -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

diff --git a/lib/icinga/timeperiod.ti b/lib/icinga/timeperiod.ti
@@ -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

diff --git a/lib/icinga/user.ti b/lib/icinga/user.ti
@@ -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

diff --git a/lib/icinga/usergroup.ti b/lib/icinga/usergroup.ti
@@ -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

diff --git a/lib/icingadb/icingadb.cpp b/lib/icingadb/icingadb.cpp
@@ -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");
 
@@ -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()));

diff --git a/lib/icingadb/icingadb.hpp b/lib/icingadb/icingadb.hpp
@@ -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>

diff --git a/tools/mkclass/classcompiler.cpp b/tools/mkclass/classcompiler.cpp
@@ -445,8 +445,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 std::mutex m_FieldsMutex;" << std::endl << std::endl;
+	}
+
+	m_Header << "public:" << std::endl
 		<< "\t" << "DECLARE_PTR_TYPEDEFS(ObjectImpl<" << klass.Name << ">);" << std::endl << std::endl;
 
 	/* Validate */
@@ -806,7 +812,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;
 
@@ -844,7 +850,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;