Rewrite Checkable::GetAllChildrenInternal() method

The previous wasn't per-se wrong, but it was way too inefficient. With
this commit each and every Checkable is going to be visited only once,
and we won't traverse the same Checkable's children multiple times
somewhere in the dependency chain.

Author: yhabteab

lib/icinga/checkable-dependency.cpp

@@ -314,7 +314,7 @@ size_t Checkable::GetAllChildrenCount() const
 
 std::set<Checkable::Ptr> Checkable::GetAllChildren() const
 {
-	std::set<Checkable::Ptr> children = GetChildren();
+	std::set<Checkable::Ptr> children;
 
 	GetAllChildrenInternal(children, 0);
 
@@ -327,31 +327,23 @@ std::set<Checkable::Ptr> Checkable::GetAllChildren() const
  * Note, this function performs a recursive call chain traversing all the children of the current Checkable
  * up to a certain limit (256). When that limit is reached, it will log a warning message and abort the operation.
  *
- * @param children - The set of children to be filled with all the children of the current Checkable.
+ * @param seenChildren - A container to store all the traversed children into.
  * @param level - The current level of recursion.
  */
-void Checkable::GetAllChildrenInternal(std::set<Checkable::Ptr>& children, int level) const
+void Checkable::GetAllChildrenInternal(std::set<Checkable::Ptr>& seenChildren, int level) const
 {
 	if (level > l_MaxDependencyRecursionLevel) {
 		Log(LogWarning, "Checkable")
 			<< "Too many nested dependencies (>" << l_MaxDependencyRecursionLevel << ") for checkable '" << GetName() << "': aborting traversal.";
 		return ;
 	}
 
-	std::set<Checkable::Ptr> localChildren;
-
-	for (const Checkable::Ptr& checkable : children) {
-		if (auto cChildren(checkable->GetChildren()); !cChildren.empty()) {
-			GetAllChildrenInternal(cChildren, level + 1);
-			localChildren.insert(cChildren.begin(), cChildren.end());
-		}
-
-		if (level != 0) { // Recursion level 0 is the initiator, so checkable is already in the set.
-			localChildren.insert(checkable);
+	for (const Checkable::Ptr& checkable : GetChildren()) {
+		if (seenChildren.find(checkable) == seenChildren.end()) {
+			seenChildren.emplace(checkable);
+			checkable->GetAllChildrenInternal(seenChildren, level + 1);
 		}
 	}
-
-	children.insert(localChildren.begin(), localChildren.end());
 }
 
 /**

lib/icinga/checkable.hpp

@@ -263,7 +263,7 @@ class Checkable : public ObjectImpl<Checkable>
 	std::unordered_set<intrusive_ptr<DependencyGroup>, HashDependencyGroup, EqualDependencyGroups> m_DependencyGroups;
 	std::set<intrusive_ptr<Dependency> > m_ReverseDependencies;
 
-	void GetAllChildrenInternal(std::set<Checkable::Ptr>& children, int level = 0) const;
+	void GetAllChildrenInternal(std::set<Checkable::Ptr>& seenChildren, int level = 0) const;
 
 	/* Flapping */
 	static const std::map<String, int> m_FlappingStateFilterMap;