Merge pull request #9543 from Icinga/apply-rules-lookup

Lookup apply rules faster by Type*, not String and by map instead of ==/!=

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;
@@ -75,7 +70,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(
+		name, expression, filter, package, fkvar, fvvar, fterm, ignoreOnError, di, scope
+	));
 }
 
 bool ApplyRule::EvaluateFilter(ScriptFrame& frame) const
@@ -111,12 +118,14 @@ bool ApplyRule::IsValidTargetType(const String& sourceType, const String& target
 	return false;
 }
 
-std::vector<String> ApplyRule::GetTargetTypes(const String& sourceType)
+const std::vector<String>& ApplyRule::GetTargetTypes(const String& sourceType)
 {
 	auto it = m_Types.find(sourceType);
 
-	if (it == m_Types.end())
+	if (it == m_Types.end()) {
-		return std::vector<String>();
+		static const std::vector<String> noTypes;
+		return noTypes;
+	}
 
 	return it->second;
 }
@@ -131,23 +140,33 @@ bool ApplyRule::HasMatches() const
 	return m_HasMatches;
 }
 
-std::vector<ApplyRule>& ApplyRule::GetRules(const String& type)
+std::vector<ApplyRule>& ApplyRule::GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType)
 {
-	auto it = m_Rules.find(type);
+	auto perSourceType (m_Rules.find(sourceType.get()));
-	if (it == m_Rules.end()) {
+
-		static std::vector<ApplyRule> emptyList;
+	if (perSourceType != m_Rules.end()) {
-		return emptyList;
+		auto perTargetType (perSourceType->second.find(targetType.get()));
+
+		if (perTargetType != perSourceType->second.end()) {
+			return perTargetType->second;
+		}
 	}
-	return it->second;
+
+	static std::vector<ApplyRule> noRules;
+	return noRules;
 }
 
 void ApplyRule::CheckMatches(bool silent)
 {
-	for (const RuleMap::value_type& kv : m_Rules) {
+	for (auto& perSourceType : m_Rules) {
-		for (const ApplyRule& rule : kv.second) {
+		for (auto& perTargetType : perSourceType.second) {
-			if (!rule.HasMatches() && !silent)
+			for (auto& rule : perTargetType.second) {
-				Log(LogWarning, "ApplyRule")
+				if (!rule.HasMatches() && !silent) {
-					<< "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '" << kv.first << "' does not match anywhere!";
+					Log(LogWarning, "ApplyRule")
+						<< "Apply rule '" << rule.GetName() << "' (" << rule.GetDebugInfo() << ") for type '"
+						<< perSourceType.first->GetName() << "' does not match anywhere!";
+				}
+			}
 		}
 	}
 }

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 <unordered_map>
 
 namespace icinga
 {
@@ -17,9 +19,8 @@ class ApplyRule
 {
 public:
 	typedef std::map<String, std::vector<String> > TypeMap;
-	typedef std::map<String, std::vector<ApplyRule> > RuleMap;
+	typedef std::unordered_map<Type*, std::unordered_map<Type*, std::vector<ApplyRule>>> RuleMap;
 
-	String GetTargetType() const;
 	String GetName() const;
 	Expression::Ptr GetExpression() const;
 	Expression::Ptr GetFilter() const;
@@ -38,17 +39,16 @@ 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<ApplyRule>& GetRules(const String& type);
+	static std::vector<ApplyRule>& GetRules(const Type::Ptr& sourceType, const Type::Ptr& targetType);
 
 	static void RegisterType(const String& sourceType, const std::vector<String>& targetTypes);
 	static bool IsValidSourceType(const String& sourceType);
 	static bool IsValidTargetType(const String& sourceType, const String& targetType);
-	static std::vector<String> GetTargetTypes(const String& sourceType);
+	static const std::vector<String>& GetTargetTypes(const String& sourceType);
 
 	static void CheckMatches(bool silent);
 
 private:
-	String m_TargetType;
 	String m_Name;
 	Expression::Ptr m_Expression;
 	Expression::Ptr m_Filter;
@@ -64,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);
 };

lib/config/config_parser.yy

@@ -1165,7 +1165,7 @@ apply:
 
 		if (!ApplyRule::IsValidTargetType(type, target)) {
 			if (target == "") {
-				std::vector<String> types = ApplyRule::GetTargetTypes(type);
+				auto& types (ApplyRule::GetTargetTypes(type));
 				String typeNames;
 
 				for (std::vector<String>::size_type i = 0; i < types.size(); i++) {

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")) {
+	for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Host::TypeInstance)) {
-		if (rule.GetTargetType() != "Host")
-			continue;
-
 		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")) {
+	for (auto& rule : ApplyRule::GetRules(Dependency::TypeInstance, Service::TypeInstance)) {
-		if (rule.GetTargetType() != "Service")
-			continue;
-
 		if (EvaluateApplyRule(service, rule))
 			rule.AddMatch();
 	}

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")) {
+	for (auto& rule : ApplyRule::GetRules(Notification::TypeInstance, Service::TypeInstance)) {
-		if (rule.GetTargetType() != "Service")
-			continue;
-
 		if (EvaluateApplyRule(service, rule))
 			rule.AddMatch();
 	}

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")) {
+	for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Host::TypeInstance)) {
-		if (rule.GetTargetType() != "Host")
-			continue;
-
 		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")) {
+	for (auto& rule : ApplyRule::GetRules(ScheduledDowntime::TypeInstance, Service::TypeInstance)) {
-		if (rule.GetTargetType() != "Service")
-			continue;
-
 		if (EvaluateApplyRule(service, rule))
 			rule.AddMatch();
 	}

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();
 	}