From 7f50955674c2e560f6c4e3d7c478861c404876d4 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 19 Oct 2022 13:42:33 +0200 Subject: [PATCH 1/3] ApplyRule::GetTargetTypes(): return by const ref not to malloc() --- lib/config/applyrule.cpp | 8 +++++--- lib/config/applyrule.hpp | 2 +- lib/config/config_parser.yy | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index 5042eb6b6..e05bce7b4 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -111,12 +111,14 @@ bool ApplyRule::IsValidTargetType(const String& sourceType, const String& target return false; } -std::vector ApplyRule::GetTargetTypes(const String& sourceType) +const std::vector& ApplyRule::GetTargetTypes(const String& sourceType) { auto it = m_Types.find(sourceType); - if (it == m_Types.end()) - return std::vector(); + if (it == m_Types.end()) { + static const std::vector noTypes; + return noTypes; + } return it->second; } diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 303b259e1..1d6e65a13 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -43,7 +43,7 @@ public: static void RegisterType(const String& sourceType, const std::vector& targetTypes); static bool IsValidSourceType(const String& sourceType); static bool IsValidTargetType(const String& sourceType, const String& targetType); - static std::vector GetTargetTypes(const String& sourceType); + static const std::vector& GetTargetTypes(const String& sourceType); static void CheckMatches(bool silent); diff --git a/lib/config/config_parser.yy b/lib/config/config_parser.yy index 8fa10e71f..939681e68 100644 --- a/lib/config/config_parser.yy +++ b/lib/config/config_parser.yy @@ -1165,7 +1165,7 @@ apply: if (!ApplyRule::IsValidTargetType(type, target)) { if (target == "") { - std::vector types = ApplyRule::GetTargetTypes(type); + auto& types (ApplyRule::GetTargetTypes(type)); String typeNames; for (std::vector::size_type i = 0; i < types.size(); i++) { From fcedb01d0d58120f948097624e7f3bc369a2447a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 17 Oct 2022 16:54:34 +0200 Subject: [PATCH 2/3] Lookup apply rules faster by Type*, not String and by map instead of ==/!= 1. The lookup of apply rules per source type now implies no String(const char*) (no malloc()) and just pointer (uint64) comparisions 2. Apply rules are now also grouped by target type via a nested map, that obsoletes checking the target type while iterating over all rules per source type --- lib/config/applyrule.cpp | 46 +++++++++++++++++++------- lib/config/applyrule.hpp | 6 ++-- lib/icinga/dependency-apply.cpp | 10 ++---- lib/icinga/notification-apply.cpp | 10 ++---- lib/icinga/scheduleddowntime-apply.cpp | 10 ++---- lib/icinga/service-apply.cpp | 2 +- 6 files changed, 45 insertions(+), 39 deletions(-) diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index e05bce7b4..3c2e6f448 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -75,7 +75,19 @@ void ApplyRule::AddRule(const String& sourceType, const String& targetType, cons const Expression::Ptr& expression, const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope) { - m_Rules[sourceType].push_back(ApplyRule(targetType, name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope)); + auto actualTargetType (&targetType); + + if (*actualTargetType == "") { + auto& targetTypes (GetTargetTypes(sourceType)); + + if (targetTypes.size() == 1u) { + actualTargetType = &targetTypes[0]; + } + } + + m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()].emplace_back(ApplyRule( + targetType, name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope + )); } bool ApplyRule::EvaluateFilter(ScriptFrame& frame) const @@ -133,23 +145,33 @@ bool ApplyRule::HasMatches() const return m_HasMatches; } -std::vector& ApplyRule::GetRules(const String& type) +std::vector& ApplyRule::GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType) { - auto it = m_Rules.find(type); - if (it == m_Rules.end()) { - static std::vector emptyList; - return emptyList; + auto perSourceType (m_Rules.find(sourceType.get())); + + if (perSourceType != m_Rules.end()) { + auto perTargetType (perSourceType->second.find(targetType.get())); + + if (perTargetType != perSourceType->second.end()) { + return perTargetType->second; + } } - return it->second; + + static std::vector noRules; + return noRules; } void ApplyRule::CheckMatches(bool silent) { - for (const RuleMap::value_type& kv : m_Rules) { - for (const ApplyRule& rule : kv.second) { - if (!rule.HasMatches() && !silent) - Log(LogWarning, "ApplyRule") - << "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" << kv.first << "' does not match anywhere!"; + for (auto& perSourceType : m_Rules) { + for (auto& perTargetType : perSourceType.second) { + for (auto& rule : perTargetType.second) { + if (!rule.HasMatches() && !silent) { + Log(LogWarning, "ApplyRule") + << "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" + << perSourceType.first->GetName() << "' does not match anywhere!"; + } + } } } } diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 1d6e65a13..63db2f691 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -6,6 +6,8 @@ #include "config/i2-config.hpp" #include "config/expression.hpp" #include "base/debuginfo.hpp" +#include "base/type.hpp" +#include namespace icinga { @@ -17,7 +19,7 @@ class ApplyRule { public: typedef std::map > TypeMap; - typedef std::map > RuleMap; + typedef std::unordered_map>> RuleMap; String GetTargetType() const; String GetName() const; @@ -38,7 +40,7 @@ public: static void AddRule(const String& sourceType, const String& targetType, const String& name, const Expression::Ptr& expression, const Expression::Ptr& filter, const String& package, const String& fkvar, const String& fvvar, const Expression::Ptr& fterm, bool ignoreOnError, const DebugInfo& di, const Dictionary::Ptr& scope); - static std::vector& GetRules(const String& type); + static std::vector& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType); static void RegisterType(const String& sourceType, const std::vector& targetTypes); static bool IsValidSourceType(const String& sourceType); diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 9830a1dac..07ab31254 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -136,10 +136,7 @@ void Dependency::EvaluateApplyRules(const Host::Ptr& host) { CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("Dependency")) { - if (rule.GetTargetType() != "Host") - continue; - + for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Host::TypeInstance)) { if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } @@ -149,10 +146,7 @@ void Dependency::EvaluateApplyRules(const Service::Ptr& service) { CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("Dependency")) { - if (rule.GetTargetType() != "Service") - continue; - + for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Service::TypeInstance)) { if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 5fbc22190..61d219215 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -135,11 +135,8 @@ void Notification::EvaluateApplyRules(const Host::Ptr& host) { CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("Notification")) + for (auto& rule : ApplyRule::GetRules(Notification::TypeInstance, Host::TypeInstance)) { - if (rule.GetTargetType() != "Host") - continue; - if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } @@ -149,10 +146,7 @@ void Notification::EvaluateApplyRules(const Service::Ptr& service) { CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("Notification")) { - if (rule.GetTargetType() != "Service") - continue; - + for (auto& rule : ApplyRule::GetRules(Notification::TypeInstance, Service::TypeInstance)) { if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 36b33cfe4..0dfaf1b09 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -134,10 +134,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Host::Ptr& host) { CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("ScheduledDowntime")) { - if (rule.GetTargetType() != "Host") - continue; - + for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Host::TypeInstance)) { if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } @@ -147,10 +144,7 @@ void ScheduledDowntime::EvaluateApplyRules(const Service::Ptr& service) { CONTEXT("Evaluating 'apply' rules for service '" + service->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("ScheduledDowntime")) { - if (rule.GetTargetType() != "Service") - continue; - + for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Service::TypeInstance)) { if (EvaluateApplyRule(service, rule)) rule.AddMatch(); } diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 760c94617..24a19c956 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -123,7 +123,7 @@ void Service::EvaluateApplyRules(const Host::Ptr& host) { CONTEXT("Evaluating 'apply' rules for host '" + host->GetName() + "'"); - for (ApplyRule& rule : ApplyRule::GetRules("Service")) { + for (auto& rule : ApplyRule::GetRules(Service::TypeInstance, Host::TypeInstance)) { if (EvaluateApplyRule(host, rule)) rule.AddMatch(); } From cf3e02243e3ceac46ca6aff045a1003432e37916 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 18 Oct 2022 12:40:09 +0200 Subject: [PATCH 3/3] Remove unused ApplyRule#m_TargetType --- lib/config/applyrule.cpp | 11 +++-------- lib/config/applyrule.hpp | 4 +--- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/config/applyrule.cpp b/lib/config/applyrule.cpp index 3c2e6f448..363dce90b 100644 --- a/lib/config/applyrule.cpp +++ b/lib/config/applyrule.cpp @@ -9,18 +9,13 @@ using namespace icinga; ApplyRule::RuleMap ApplyRule::m_Rules; ApplyRule::TypeMap ApplyRule::m_Types; -ApplyRule::ApplyRule(String targetType, String name, Expression::Ptr expression, +ApplyRule::ApplyRule(String name, Expression::Ptr expression, Expression::Ptr filter, String package, String fkvar, String fvvar, Expression::Ptr fterm, bool ignoreOnError, DebugInfo di, Dictionary::Ptr scope) - : m_TargetType(std::move(targetType)), m_Name(std::move(name)), m_Expression(std::move(expression)), m_Filter(std::move(filter)), m_Package(std::move(package)), m_FKVar(std::move(fkvar)), + : m_Name(std::move(name)), m_Expression(std::move(expression)), m_Filter(std::move(filter)), m_Package(std::move(package)), m_FKVar(std::move(fkvar)), m_FVVar(std::move(fvvar)), m_FTerm(std::move(fterm)), m_IgnoreOnError(ignoreOnError), m_DebugInfo(std::move(di)), m_Scope(std::move(scope)), m_HasMatches(false) { } -String ApplyRule::GetTargetType() const -{ - return m_TargetType; -} - String ApplyRule::GetName() const { return m_Name; @@ -86,7 +81,7 @@ void ApplyRule::AddRule(const String& sourceType, const String& targetType, cons } m_Rules[Type::GetByName(sourceType).get()][Type::GetByName(*actualTargetType).get()].emplace_back(ApplyRule( - targetType, name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope + name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope )); } diff --git a/lib/config/applyrule.hpp b/lib/config/applyrule.hpp index 63db2f691..10520bfbd 100644 --- a/lib/config/applyrule.hpp +++ b/lib/config/applyrule.hpp @@ -21,7 +21,6 @@ public: typedef std::map > TypeMap; typedef std::unordered_map>> RuleMap; - String GetTargetType() const; String GetName() const; Expression::Ptr GetExpression() const; Expression::Ptr GetFilter() const; @@ -50,7 +49,6 @@ public: static void CheckMatches(bool silent); private: - String m_TargetType; String m_Name; Expression::Ptr m_Expression; Expression::Ptr m_Filter; @@ -66,7 +64,7 @@ private: static TypeMap m_Types; static RuleMap m_Rules; - ApplyRule(String targetType, String name, Expression::Ptr expression, + ApplyRule(String name, Expression::Ptr expression, Expression::Ptr filter, String package, String fkvar, String fvvar, Expression::Ptr fterm, bool ignoreOnError, DebugInfo di, Dictionary::Ptr scope); };