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.
This commit is contained in:
Yonas Habteab 2025-01-31 16:30:13 +01:00
parent e0ce0ccff6
commit c465f45200
2 changed files with 10 additions and 18 deletions

View File

@ -8,7 +8,7 @@
using namespace icinga;
/**
* The maximum number of dependency recursion levels allowed.
* The maximum number of allowed dependency recursion levels.
*
* This is a subjective limit how deep the dependency tree should be allowed to go, as anything beyond this level
* is just madness and will likely result in a stack overflow or other undefined behavior.
@ -197,7 +197,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);
@ -210,29 +210,21 @@ 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 ;
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 (auto [_, inserted] = seenChildren.insert(checkable); inserted) {
seenChildren.emplace(checkable);
checkable->GetAllChildrenInternal(seenChildren, level + 1);
}
}
children.insert(localChildren.begin(), localChildren.end());
}

View File

@ -249,7 +249,7 @@ private:
std::set<intrusive_ptr<Dependency> > m_Dependencies;
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;