Simplify dependency group registration

Co-Authored-By: Yonas Habteab <yonas.habteab@icinga.com>

lib/icinga/checkable-dependency.cpp

@@ -15,29 +15,91 @@ using namespace icinga;
  */
 static constexpr int l_MaxDependencyRecursionLevel(256);
 
-void Checkable::AddDependencyGroup(const DependencyGroup::Ptr& dependencyGroup)
-{
-	std::unique_lock lock(m_DependencyMutex);
-	m_DependencyGroups.insert(dependencyGroup);
-}
-
-void Checkable::RemoveDependencyGroup(const DependencyGroup::Ptr& dependencyGroup)
-{
-	std::unique_lock lock(m_DependencyMutex);
-	m_DependencyGroups.erase(dependencyGroup);
-}
-
 std::vector<DependencyGroup::Ptr> Checkable::GetDependencyGroups() const
 {
 	std::lock_guard lock(m_DependencyMutex);
-	return {m_DependencyGroups.begin(), m_DependencyGroups.end()};
+	std::vector<DependencyGroup::Ptr> dependencyGroups;
+	for (const auto& [_, dependencyGroup] : m_DependencyGroups) {
+		dependencyGroups.emplace_back(dependencyGroup);
+	}
+	return dependencyGroups;
+}
+
+/**
+ * Get the key for the provided dependency group.
+ *
+ * The key is either the parent Checkable object or the redundancy group name of the dependency object.
+ * This is used to uniquely identify the dependency group within a given Checkable object.
+ *
+ * @param dependency The dependency to get the key for.
+ *
+ * @return - Returns the key for the provided dependency group.
+ */
+static std::variant<Checkable*, String> GetDependencyGroupKey(const Dependency::Ptr& dependency)
+{
+	if (auto redundancyGroup(dependency->GetRedundancyGroup()); !redundancyGroup.IsEmpty()) {
+		return std::move(redundancyGroup);
+	}
+
+	return dependency->GetParent().get();
+}
+
+/**
+ * Add the provided dependency to the current Checkable list of dependencies.
+ *
+ * @param dependency The dependency to add.
+ */
+void Checkable::AddDependency(const Dependency::Ptr& dependency)
+{
+	std::lock_guard lock(m_DependencyMutex);
+
+	auto dependencyGroupKey(GetDependencyGroupKey(dependency));
+	std::set<Dependency::Ptr> dependencies;
+	if (auto it(m_DependencyGroups.find(dependencyGroupKey)); it != m_DependencyGroups.end()) {
+		dependencies = DependencyGroup::Unregister(it->second, this);
+		m_DependencyGroups.erase(it);
+	}
+
+	dependencies.emplace(dependency);
+
+	m_DependencyGroups.emplace(
+		dependencyGroupKey,
+		DependencyGroup::Register(new DependencyGroup(dependency->GetRedundancyGroup(), dependencies))
+	);
+}
+
+/**
+ * Remove the provided dependency from the current Checkable list of dependencies.
+ *
+ * @param dependency The dependency to remove.
+ */
+void Checkable::RemoveDependency(const Dependency::Ptr& dependency)
+{
+	std::lock_guard lock(m_DependencyMutex);
+
+	auto dependencyGroupKey(GetDependencyGroupKey(dependency));
+	auto it = m_DependencyGroups.find(dependencyGroupKey);
+	if (it == m_DependencyGroups.end()) {
+		return;
+	}
+
+	std::set<Dependency::Ptr> dependencies(DependencyGroup::Unregister(it->second, this));
+	m_DependencyGroups.erase(it);
+	dependencies.erase(dependency);
+
+	if (!dependencies.empty()) {
+		m_DependencyGroups.emplace(
+			dependencyGroupKey,
+			DependencyGroup::Register(new DependencyGroup(dependency->GetRedundancyGroup(), dependencies))
+		);
+	}
 }
 
 std::vector<Dependency::Ptr> Checkable::GetDependencies() const
 {
 	std::unique_lock<std::mutex> lock(m_DependencyMutex);
 	std::vector<Dependency::Ptr> dependencies;
-	for (const auto& dependencyGroup : m_DependencyGroups) {
+	for (const auto& [_, dependencyGroup] : m_DependencyGroups) {
 		auto tmpDependencies(dependencyGroup->GetDependenciesForChild(this));
 		dependencies.insert(dependencies.end(), tmpDependencies.begin(), tmpDependencies.end());
 	}

lib/icinga/checkable.hpp

@@ -18,6 +18,7 @@
 #include <cstdint>
 #include <functional>
 #include <limits>
+#include <variant>
 
 namespace icinga
 {
@@ -185,9 +186,9 @@ public:
 	bool IsFlapping() const;
 
 	/* Dependencies */
-	void AddDependencyGroup(const intrusive_ptr<DependencyGroup>& dependencyGroup);
-	void RemoveDependencyGroup(const intrusive_ptr<DependencyGroup>& dependencyGroup);
 	std::vector<intrusive_ptr<DependencyGroup>> GetDependencyGroups() const;
+	void AddDependency(const intrusive_ptr<Dependency>& dependency);
+	void RemoveDependency(const intrusive_ptr<Dependency>& dependency);
 	std::vector<intrusive_ptr<Dependency> > GetDependencies() const;
 	bool HasAnyDependencies() const;
 
@@ -249,7 +250,7 @@ private:
 
 	/* Dependencies */
 	mutable std::mutex m_DependencyMutex;
-	std::set<intrusive_ptr<DependencyGroup>> m_DependencyGroups;
+	std::map<std::variant<Checkable*, String>, intrusive_ptr<DependencyGroup>> m_DependencyGroups;
 	std::set<intrusive_ptr<Dependency> > m_ReverseDependencies;
 
 	void GetAllChildrenInternal(std::set<Checkable::Ptr>& seenChildren, int level = 0) const;

lib/icinga/dependency-group.cpp

@@ -9,106 +9,52 @@ std::mutex DependencyGroup::m_RegistryMutex;
 DependencyGroup::RegistryType DependencyGroup::m_Registry;
 
 /**
- * Refresh the global registry of dependency groups.
+ * Register the provided dependency group to the global dependency group registry.
  *
- * Registers the provided dependency object to an existing dependency group with the same redundancy
- * group name (if any), or creates a new one and registers it to the child Checkable and the registry.
+ * In case there is already an identical dependency group in the registry, the provided dependency group is merged
+ * with the existing one, and that group is returned. Otherwise, the provided dependency group is registered as is,
+ * and it's returned.
  *
- * Note: This is a helper function intended for internal use only, and you should acquire the global registry mutex
- * before calling this function.
- *
- * @param dependency The dependency object to refresh the registry for.
- * @param unregister A flag indicating whether the provided dependency object should be unregistered from the registry.
+ * @param dependencyGroup The dependency group to register.
  */
-void DependencyGroup::RefreshRegistry(const Dependency::Ptr& dependency, bool unregister)
+DependencyGroup::Ptr DependencyGroup::Register(const DependencyGroup::Ptr& dependencyGroup)
 {
-	auto registerRedundancyGroup = [](const DependencyGroup::Ptr& dependencyGroup) {
-		if (auto [it, inserted](m_Registry.insert(dependencyGroup.get())); !inserted) {
-			DependencyGroup::Ptr existingGroup(*it);
-			dependencyGroup->CopyDependenciesTo(existingGroup);
-		}
-	};
-
-	// Retrieve all the dependency groups with the same redundancy group name of the provided dependency object.
-	// This allows us to shorten the lookup for the _one_ optimal group to (un)register the dependency from/to.
-	auto [begin, end] = m_Registry.get<1>().equal_range(dependency->GetRedundancyGroup());
-	for (auto it(begin); it != end; ++it) {
-		DependencyGroup::Ptr existingGroup(*it);
-		auto child(dependency->GetChild());
-		if (auto dependencies(existingGroup->GetDependenciesForChild(child.get())); !dependencies.empty()) {
-			m_Registry.erase(existingGroup->GetCompositeKey()); // Will be re-registered when needed down below.
-			if (unregister) {
-				existingGroup->RemoveDependency(dependency);
-				// Remove the connection between the child Checkable and the dependency group if it has no members
-				// left or the above removed member was the only member of the group that the child depended on.
-				if (existingGroup->IsEmpty() || dependencies.size() == 1) {
-					child->RemoveDependencyGroup(existingGroup);
-				}
-			}
-
-			size_t totalDependencies(existingGroup->GetDependenciesCount());
-			// If the existing dependency group has an identical member already, or the child Checkable of the
-			// dependency object is the only member of it (totalDependencies == dependencies.size()), we can simply
-			// add the dependency object to the existing group.
-			if (!unregister && (existingGroup->HasParentWithConfig(dependency) || totalDependencies == dependencies.size())) {
-				existingGroup->AddDependency(dependency);
-			} else if (!unregister || (dependencies.size() > 1 && totalDependencies >= dependencies.size())) {
-				// The child Checkable is going to have a new dependency group, so we must detach the existing one.
-				child->RemoveDependencyGroup(existingGroup);
-
-				Ptr replacementGroup(unregister ? nullptr : new DependencyGroup(existingGroup->GetRedundancyGroupName(), dependency));
-				for (auto& existingDependency : dependencies) {
-					if (existingDependency != dependency) {
-						existingGroup->RemoveDependency(existingDependency);
-						if (replacementGroup) {
-							replacementGroup->AddDependency(existingDependency);
-						} else {
-							replacementGroup = new DependencyGroup(existingGroup->GetRedundancyGroupName(), existingDependency);
-						}
-					}
-				}
-
-				child->AddDependencyGroup(replacementGroup);
-				registerRedundancyGroup(replacementGroup);
-			}
-
-			if (!existingGroup->IsEmpty()) {
-				registerRedundancyGroup(existingGroup);
-			}
-			return;
-		}
-	}
-
-	if (!unregister) {
-		// We couldn't find any existing dependency group to register the dependency to, so we must
-		// initiate a new one and attach it to the child Checkable and register to the global registry.
-		DependencyGroup::Ptr newGroup(new DependencyGroup(dependency->GetRedundancyGroup()));
-		newGroup->AddDependency(dependency);
-		dependency->GetChild()->AddDependencyGroup(newGroup);
-		registerRedundancyGroup(newGroup);
+	std::lock_guard lock(m_RegistryMutex);
+	if (auto [it, inserted] = m_Registry.insert(dependencyGroup); !inserted) {
+		dependencyGroup->CopyDependenciesTo(*it);
+		return *it;
 	}
+	return dependencyGroup;
 }
 
 /**
- * Register the provided dependency to the global dependency group registry.
+ * Detach the provided child Checkable from the specified dependency group.
  *
- * @param dependency The dependency to register.
+ * Unregisters all the dependency objects the child Checkable depends on from the provided dependency group and
+ * removes the dependency group from the global registry if it becomes empty afterward.
+ *
+ * @param dependencyGroup The dependency group to unregister the child Checkable from.
+ * @param child The child Checkable to detach from the dependency group.
+ *
+ * @return - Returns the dependency objects of the child Checkable that were member of the provided dependency group.
  */
-void DependencyGroup::Register(const Dependency::Ptr& dependency)
+std::set<Dependency::Ptr> DependencyGroup::Unregister(const DependencyGroup::Ptr& dependencyGroup, const Checkable::Ptr& child)
 {
 	std::lock_guard lock(m_RegistryMutex);
-	RefreshRegistry(dependency, false);
-}
+	std::vector<Dependency::Ptr> dependencies;
+	if (auto it(m_Registry.find(dependencyGroup)); it != m_Registry.end()) {
+		const auto& existingGroup(*it);
+		dependencies = existingGroup->GetDependenciesForChild(child.get());
 
-/**
- * Unregister the provided dependency from the dependency group it was member of.
- *
- * @param dependency The dependency to unregister.
- */
-void DependencyGroup::Unregister(const Dependency::Ptr& dependency)
-{
-	std::lock_guard lock(m_RegistryMutex);
-	RefreshRegistry(dependency, true);
+		for (const auto& dependency : dependencies) {
+			existingGroup->RemoveDependency(dependency);
+		}
+
+		if (existingGroup->IsEmpty()) {
+			m_Registry.erase(it);
+		}
+	}
+	return {dependencies.begin(), dependencies.end()};
 }
 
 /**
@@ -126,6 +72,13 @@ DependencyGroup::DependencyGroup(String name): m_RedundancyGroupName(std::move(n
 {
 }
 
+DependencyGroup::DependencyGroup(String name, const std::set<Dependency::Ptr>& dependencies): m_RedundancyGroupName(std::move(name))
+{
+	for (const auto& dependency : dependencies) {
+		AddDependency(dependency);
+	}
+}
+
 /**
  * Create a composite key for the provided dependency.
  *
@@ -245,17 +198,10 @@ void DependencyGroup::CopyDependenciesTo(const DependencyGroup::Ptr& dest)
 	VERIFY(this != dest); // Prevent from doing something stupid, i.e. deadlocking ourselves.
 
 	std::lock_guard lock(m_Mutex);
-	DependencyGroup::Ptr thisPtr(this); // Just in case the Checkable below was our last reference.
 	for (auto& [_, children] : m_Members) {
-		Checkable::Ptr previousChild;
-		for (auto& [checkable, dependency] : children) {
-			dest->AddDependency(dependency);
-			if (!previousChild || previousChild != checkable) {
-				previousChild = dependency->GetChild();
-				previousChild->RemoveDependencyGroup(thisPtr);
-				previousChild->AddDependencyGroup(dest);
-			}
-		}
+		std::for_each(children.begin(), children.end(), [&dest](const auto& pair) {
+			dest->AddDependency(pair.second);
+		});
 	}
 }
 

lib/icinga/dependency.cpp

@@ -251,7 +251,7 @@ void Dependency::OnAllConfigLoaded()
 	// InitChildParentReferences() has to be called before.
 	VERIFY(m_Child && m_Parent);
 
-	DependencyGroup::Register(this);
+	m_Child->AddDependency(this);
 	m_Parent->AddReverseDependency(this);
 }
 
@@ -259,7 +259,7 @@ void Dependency::Stop(bool runtimeRemoved)
 {
 	ObjectImpl<Dependency>::Stop(runtimeRemoved);
 
-	DependencyGroup::Unregister(this);
+	GetChild()->RemoveDependency(this);
 	GetParent()->RemoveReverseDependency(this);
 }
 

lib/icinga/dependency.hpp

@@ -8,9 +8,6 @@
 #include "icinga/i2-icinga.hpp"
 #include "icinga/dependency-ti.hpp"
 #include "icinga/timeperiod.hpp"
-#include <boost/multi_index_container.hpp>
-#include <boost/multi_index/hashed_index.hpp>
-#include <boost/multi_index/mem_fun.hpp>
 #include <map>
 #include <tuple>
 #include <unordered_map>
@@ -136,9 +133,10 @@ public:
 	using MembersMap = std::map<CompositeKeyType, MemberValueType>;
 
 	explicit DependencyGroup(String name);
+	DependencyGroup(String name, const std::set<Dependency::Ptr>& dependencies);
 
-	static void Register(const Dependency::Ptr& dependency);
-	static void Unregister(const Dependency::Ptr& dependency);
+	static DependencyGroup::Ptr Register(const DependencyGroup::Ptr& dependencyGroup);
+	static std::set<Dependency::Ptr> Unregister(const DependencyGroup::Ptr& dependencyGroup, const Checkable::Ptr& child);
 	static size_t GetRegistrySize();
 
 	static CompositeKeyType MakeCompositeKeyFor(const Dependency::Ptr& dependency);
@@ -176,7 +174,29 @@ protected:
 	void RemoveDependency(const Dependency::Ptr& dependency);
 	void CopyDependenciesTo(const DependencyGroup::Ptr& dest);
 
-	static void RefreshRegistry(const Dependency::Ptr& dependency, bool unregister);
+	struct Hash
+	{
+		size_t operator()(const DependencyGroup::Ptr& dependencyGroup) const
+		{
+			size_t hash = 0;
+			for (const auto& [key, group] : dependencyGroup->m_Members) {
+				boost::hash_combine(hash, key);
+			}
+			return hash;
+		}
+	};
+
+	struct Equal
+	{
+		bool operator()(const DependencyGroup::Ptr& lhs, const DependencyGroup::Ptr& rhs) const
+		{
+			return std::equal(
+				lhs->m_Members.begin(), lhs->m_Members.end(),
+				rhs->m_Members.begin(), rhs->m_Members.end(),
+				[](const auto& l, const auto& r) { return l.first == r.first; }
+			);
+		}
+	};
 
 private:
 	mutable std::mutex m_Mutex;
@@ -193,25 +213,7 @@ private:
 	String m_RedundancyGroupName;
 	MembersMap m_Members;
 
-	using RegistryType = boost::multi_index_container<
-		DependencyGroup*, // The type of the elements stored in the container.
-		boost::multi_index::indexed_by<
-			// This unique index allows to search/erase dependency groups by their composite key in an efficient manner.
-			boost::multi_index::hashed_unique<
-				boost::multi_index::mem_fun<DependencyGroup, String, &DependencyGroup::GetCompositeKey>,
-				std::hash<String>
-			>,
-			// This non-unique index allows to search for dependency groups by their name, and reduces the overall
-			// runtime complexity. Without this index, we would have to iterate over all elements to find the one
-			// with the desired members and since containers don't allow erasing elements while iterating, we would
-			// have to copy each of them to a temporary container, and then erase and reinsert them back to the original
-			// container. This produces way too much overhead, and slows down the startup time of Icinga 2 significantly.
-			boost::multi_index::hashed_non_unique<
-				boost::multi_index::const_mem_fun<DependencyGroup, const String&, &DependencyGroup::GetName>,
-				std::hash<String>
-			>
-		>
-	>;
+	using RegistryType = std::unordered_set<DependencyGroup::Ptr, Hash, Equal>;
 
 	// The global registry of dependency groups.
 	static std::mutex m_RegistryMutex;