From 8c6bb1f46c2025206bb270f1baa390a8981337d9 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 8 Nov 2022 13:26:27 +0100 Subject: [PATCH 1/4] T::EvaluateApplyRule(): don't squish non-for apply rules into apply-for logic to K. I. S. S. and avoid malloc(). --- lib/icinga/dependency-apply.cpp | 56 ++++++++++++------------ lib/icinga/notification-apply.cpp | 56 ++++++++++++------------ lib/icinga/scheduleddowntime-apply.cpp | 56 ++++++++++++------------ lib/icinga/service-apply.cpp | 60 +++++++++++++------------- 4 files changed, 111 insertions(+), 117 deletions(-) diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 8681c4332..cd4aff3fc 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -79,52 +79,50 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR if (service) frame.Locals->Set("service", service); - Value vinstances; + bool match = false; if (rule.GetFTerm()) { + Value vinstances; + try { vinstances = rule.GetFTerm()->Evaluate(frame); } catch (const std::exception&) { /* Silently ignore errors here and assume there are no instances. */ return false; } - } else { - vinstances = new Array({ "" }); - } - bool match = false; + if (vinstances.IsObjectType()) { + if (!rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); - if (vinstances.IsObjectType()) { - if (!rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); + Array::Ptr arr = vinstances; - Array::Ptr arr = vinstances; + ObjectLock olock(arr); + for (const Value& instance : arr) { + String name = rule.GetName(); - ObjectLock olock(arr); - for (const Value& instance : arr) { - String name = rule.GetName(); - - if (!rule.GetFKVar().IsEmpty()) { frame.Locals->Set(rule.GetFKVar(), instance); name += instance; + + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) + match = true; } + } else if (vinstances.IsObjectType()) { + if (rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) - match = true; - } - } else if (vinstances.IsObjectType()) { - if (rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - - Dictionary::Ptr dict = vinstances; - - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) - match = true; + Dictionary::Ptr dict = vinstances; + + for (const String& key : dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + match = true; + } } + } else if (EvaluateApplyRuleInstance(checkable, rule.GetName(), frame, rule, skipFilter)) { + match = true; } return match; diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index f5b37643b..51408fb24 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -78,52 +78,50 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl if (service) frame.Locals->Set("service", service); - Value vinstances; + bool match = false; if (rule.GetFTerm()) { + Value vinstances; + try { vinstances = rule.GetFTerm()->Evaluate(frame); } catch (const std::exception&) { /* Silently ignore errors here and assume there are no instances. */ return false; } - } else { - vinstances = new Array({ "" }); - } - bool match = false; + if (vinstances.IsObjectType()) { + if (!rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); - if (vinstances.IsObjectType()) { - if (!rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); + Array::Ptr arr = vinstances; - Array::Ptr arr = vinstances; + ObjectLock olock(arr); + for (const Value& instance : arr) { + String name = rule.GetName(); - ObjectLock olock(arr); - for (const Value& instance : arr) { - String name = rule.GetName(); - - if (!rule.GetFKVar().IsEmpty()) { frame.Locals->Set(rule.GetFKVar(), instance); name += instance; + + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) + match = true; } + } else if (vinstances.IsObjectType()) { + if (rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) - match = true; - } - } else if (vinstances.IsObjectType()) { - if (rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - - Dictionary::Ptr dict = vinstances; - - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) - match = true; + Dictionary::Ptr dict = vinstances; + + for (const String& key : dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + match = true; + } } + } else if (EvaluateApplyRuleInstance(checkable, rule.GetName(), frame, rule, skipFilter)) { + match = true; } return match; diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 4f8aa471f..7aea69ebf 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -77,52 +77,50 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const if (service) frame.Locals->Set("service", service); - Value vinstances; + bool match = false; if (rule.GetFTerm()) { + Value vinstances; + try { vinstances = rule.GetFTerm()->Evaluate(frame); } catch (const std::exception&) { /* Silently ignore errors here and assume there are no instances. */ return false; } - } else { - vinstances = new Array({ "" }); - } - bool match = false; + if (vinstances.IsObjectType()) { + if (!rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); - if (vinstances.IsObjectType()) { - if (!rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); + Array::Ptr arr = vinstances; - Array::Ptr arr = vinstances; + ObjectLock olock(arr); + for (const Value& instance : arr) { + String name = rule.GetName(); - ObjectLock olock(arr); - for (const Value& instance : arr) { - String name = rule.GetName(); - - if (!rule.GetFKVar().IsEmpty()) { frame.Locals->Set(rule.GetFKVar(), instance); name += instance; + + if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) + match = true; } + } else if (vinstances.IsObjectType()) { + if (rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) - match = true; - } - } else if (vinstances.IsObjectType()) { - if (rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - - Dictionary::Ptr dict = vinstances; - - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) - match = true; + Dictionary::Ptr dict = vinstances; + + for (const String& key : dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + match = true; + } } + } else if (EvaluateApplyRuleInstance(checkable, rule.GetName(), frame, rule, skipFilter)) { + match = true; } return match; diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index 4419e0b34..d6d6c1b35 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -66,52 +66,52 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bo rule.GetScope()->CopyTo(frame.Locals); frame.Locals->Set("host", host); - Value vinstances; + bool match = false; if (rule.GetFTerm()) { + Value vinstances; + try { vinstances = rule.GetFTerm()->Evaluate(frame); } catch (const std::exception&) { /* Silently ignore errors here and assume there are no instances. */ return false; } - } else { - vinstances = new Array({ "" }); - } - bool match = false; + if (vinstances.IsObjectType()) { + if (!rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); - if (vinstances.IsObjectType()) { - if (!rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Dictionary iterator requires value to be a dictionary.", di)); + Array::Ptr arr = vinstances; - Array::Ptr arr = vinstances; + ObjectLock olock(arr); + for (const Value& instance : arr) { + String name = rule.GetName(); - ObjectLock olock(arr); - for (const Value& instance : arr) { - String name = rule.GetName(); + if (!rule.GetFKVar().IsEmpty()) { + frame.Locals->Set(rule.GetFKVar(), instance); + name += instance; + } - if (!rule.GetFKVar().IsEmpty()) { - frame.Locals->Set(rule.GetFKVar(), instance); - name += instance; + if (EvaluateApplyRuleInstance(host, name, frame, rule, skipFilter)) + match = true; } + } else if (vinstances.IsObjectType()) { + if (rule.GetFVVar().IsEmpty()) + BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - if (EvaluateApplyRuleInstance(host, name, frame, rule, skipFilter)) - match = true; - } - } else if (vinstances.IsObjectType()) { - if (rule.GetFVVar().IsEmpty()) - BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); - - Dictionary::Ptr dict = vinstances; - - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); - - if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule, skipFilter)) - match = true; + Dictionary::Ptr dict = vinstances; + + for (const String& key : dict->GetKeys()) { + frame.Locals->Set(rule.GetFKVar(), key); + frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + + if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule, skipFilter)) + match = true; + } } + } else if (EvaluateApplyRuleInstance(host, rule.GetName(), frame, rule, skipFilter)) { + match = true; } return match; From 957be31cd79b8823e59a739a473bb151a8c8c72d Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 8 Nov 2022 14:45:52 +0100 Subject: [PATCH 2/4] T::EvaluateApplyRule(): K. I. S. S. while iterating over a dict to also save locks and malloc(). --- lib/icinga/dependency-apply.cpp | 9 +++++---- lib/icinga/notification-apply.cpp | 9 +++++---- lib/icinga/scheduleddowntime-apply.cpp | 9 +++++---- lib/icinga/service-apply.cpp | 9 +++++---- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index cd4aff3fc..64a700d7e 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -112,12 +112,13 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Dictionary::Ptr dict = vinstances; + ObjectLock olock (dict); - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + for (auto& kv : dict) { + frame.Locals->Set(rule.GetFKVar(), kv.first); + frame.Locals->Set(rule.GetFVVar(), kv.second); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; } } diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 51408fb24..24e6f5fbf 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -111,12 +111,13 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Dictionary::Ptr dict = vinstances; + ObjectLock olock (dict); - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + for (auto& kv : dict) { + frame.Locals->Set(rule.GetFKVar(), kv.first); + frame.Locals->Set(rule.GetFVVar(), kv.second); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; } } diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 7aea69ebf..558dcfd86 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -110,12 +110,13 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Dictionary::Ptr dict = vinstances; + ObjectLock olock (dict); - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + for (auto& kv : dict) { + frame.Locals->Set(rule.GetFKVar(), kv.first); + frame.Locals->Set(rule.GetFVVar(), kv.second); - if (EvaluateApplyRuleInstance(checkable, rule.GetName() + key, frame, rule, skipFilter)) + if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; } } diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index d6d6c1b35..ab472a47a 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -101,12 +101,13 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bo BOOST_THROW_EXCEPTION(ScriptError("Array iterator requires value to be an array.", di)); Dictionary::Ptr dict = vinstances; + ObjectLock olock (dict); - for (const String& key : dict->GetKeys()) { - frame.Locals->Set(rule.GetFKVar(), key); - frame.Locals->Set(rule.GetFVVar(), dict->Get(key)); + for (auto& kv : dict) { + frame.Locals->Set(rule.GetFKVar(), kv.first); + frame.Locals->Set(rule.GetFVVar(), kv.second); - if (EvaluateApplyRuleInstance(host, rule.GetName() + key, frame, rule, skipFilter)) + if (EvaluateApplyRuleInstance(host, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; } } From 805c78a7b9bec6b5c875a94d1b86d38b8297d605 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 8 Nov 2022 16:12:42 +0100 Subject: [PATCH 3/4] Cache locals of script frame used for assign where eval to avoid malloc(). This BREAKS assign where locals.x = 1. --- lib/icinga/checkable.cpp | 9 +++++++++ lib/icinga/checkable.hpp | 6 ++++++ lib/icinga/dependency-apply.cpp | 28 +++++++++++++++----------- lib/icinga/host.cpp | 5 +++++ lib/icinga/host.hpp | 2 ++ lib/icinga/notification-apply.cpp | 28 +++++++++++++++----------- lib/icinga/scheduleddowntime-apply.cpp | 28 +++++++++++++++----------- lib/icinga/service-apply.cpp | 24 +++++++++++++++------- lib/icinga/service.cpp | 8 ++++++++ lib/icinga/service.hpp | 2 ++ 10 files changed, 97 insertions(+), 43 deletions(-) diff --git a/lib/icinga/checkable.cpp b/lib/icinga/checkable.cpp index dbe73fe55..3cde81537 100644 --- a/lib/icinga/checkable.cpp +++ b/lib/icinga/checkable.cpp @@ -328,3 +328,12 @@ String Checkable::StateTypeToString(StateType type) return type == StateTypeSoft ? "SOFT" : "HARD"; } +Dictionary::Ptr Checkable::GetFrozenLocalsForApply() +{ + if (!m_FrozenLocalsForApply) { + m_FrozenLocalsForApply = MakeLocalsForApply(); + m_FrozenLocalsForApply->Freeze(); + } + + return m_FrozenLocalsForApply; +} diff --git a/lib/icinga/checkable.hpp b/lib/icinga/checkable.hpp index 2ccc87073..6c1add9ce 100644 --- a/lib/icinga/checkable.hpp +++ b/lib/icinga/checkable.hpp @@ -216,12 +216,18 @@ public: static int GetPendingChecks(); static void AquirePendingCheckSlot(int maxPendingChecks); + Dictionary::Ptr GetFrozenLocalsForApply(); + protected: void Start(bool runtimeCreated) override; void OnConfigLoaded() override; void OnAllConfigLoaded() override; + virtual Dictionary::Ptr MakeLocalsForApply() = 0; + private: + Dictionary::Ptr m_FrozenLocalsForApply; + mutable std::mutex m_CheckableMutex; bool m_CheckRunning{false}; long m_SchedulingOffset; diff --git a/lib/icinga/dependency-apply.cpp b/lib/icinga/dependency-apply.cpp index 64a700d7e..62f09e7fe 100644 --- a/lib/icinga/dependency-apply.cpp +++ b/lib/icinga/dependency-apply.cpp @@ -68,16 +68,20 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR CONTEXT("Evaluating 'apply' rule (" << di << ")"); - Host::Ptr host; - Service::Ptr service; - tie(host, service) = GetHostService(checkable); + ScriptFrame frame (false); - ScriptFrame frame(true); - if (rule.GetScope()) - rule.GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); - if (service) - frame.Locals->Set("service", service); + if (rule.GetScope() || rule.GetFTerm()) { + frame.Locals = new Dictionary(); + + if (rule.GetScope()) { + rule.GetScope()->CopyTo(frame.Locals); + } + + checkable->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = checkable->GetFrozenLocalsForApply(); + } bool match = false; @@ -101,7 +105,7 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR for (const Value& instance : arr) { String name = rule.GetName(); - frame.Locals->Set(rule.GetFKVar(), instance); + frame.Locals->Set(rule.GetFKVar(), instance, true); name += instance; if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) @@ -115,8 +119,8 @@ bool Dependency::EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyR ObjectLock olock (dict); for (auto& kv : dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + frame.Locals->Set(rule.GetFKVar(), kv.first, true); + frame.Locals->Set(rule.GetFVVar(), kv.second, true); if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index 35fb25537..fb7b2300c 100644 --- a/lib/icinga/host.cpp +++ b/lib/icinga/host.cpp @@ -307,3 +307,8 @@ bool Host::ResolveMacro(const String& macro, const CheckResult::Ptr&, Value *res return false; } + +Dictionary::Ptr Host::MakeLocalsForApply() +{ + return new Dictionary({{ "host", this }}); +} diff --git a/lib/icinga/host.hpp b/lib/icinga/host.hpp index 7cacd160f..2a432b23d 100644 --- a/lib/icinga/host.hpp +++ b/lib/icinga/host.hpp @@ -54,6 +54,8 @@ protected: void CreateChildObjects(const Type::Ptr& childType) override; + Dictionary::Ptr MakeLocalsForApply() override; + private: mutable std::mutex m_ServicesMutex; std::map > m_Services; diff --git a/lib/icinga/notification-apply.cpp b/lib/icinga/notification-apply.cpp index 24e6f5fbf..78c8acd17 100644 --- a/lib/icinga/notification-apply.cpp +++ b/lib/icinga/notification-apply.cpp @@ -67,16 +67,20 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl CONTEXT("Evaluating 'apply' rule (" << di << ")"); - Host::Ptr host; - Service::Ptr service; - tie(host, service) = GetHostService(checkable); + ScriptFrame frame (false); - ScriptFrame frame(true); - if (rule.GetScope()) - rule.GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); - if (service) - frame.Locals->Set("service", service); + if (rule.GetScope() || rule.GetFTerm()) { + frame.Locals = new Dictionary(); + + if (rule.GetScope()) { + rule.GetScope()->CopyTo(frame.Locals); + } + + checkable->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = checkable->GetFrozenLocalsForApply(); + } bool match = false; @@ -100,7 +104,7 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl for (const Value& instance : arr) { String name = rule.GetName(); - frame.Locals->Set(rule.GetFKVar(), instance); + frame.Locals->Set(rule.GetFKVar(), instance, true); name += instance; if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) @@ -114,8 +118,8 @@ bool Notification::EvaluateApplyRule(const Checkable::Ptr& checkable, const Appl ObjectLock olock (dict); for (auto& kv : dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + frame.Locals->Set(rule.GetFKVar(), kv.first, true); + frame.Locals->Set(rule.GetFVVar(), kv.second, true); if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; diff --git a/lib/icinga/scheduleddowntime-apply.cpp b/lib/icinga/scheduleddowntime-apply.cpp index 558dcfd86..0442158ec 100644 --- a/lib/icinga/scheduleddowntime-apply.cpp +++ b/lib/icinga/scheduleddowntime-apply.cpp @@ -66,16 +66,20 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const CONTEXT("Evaluating 'apply' rule (" << di << ")"); - Host::Ptr host; - Service::Ptr service; - tie(host, service) = GetHostService(checkable); + ScriptFrame frame (false); - ScriptFrame frame(true); - if (rule.GetScope()) - rule.GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); - if (service) - frame.Locals->Set("service", service); + if (rule.GetScope() || rule.GetFTerm()) { + frame.Locals = new Dictionary(); + + if (rule.GetScope()) { + rule.GetScope()->CopyTo(frame.Locals); + } + + checkable->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = checkable->GetFrozenLocalsForApply(); + } bool match = false; @@ -99,7 +103,7 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const for (const Value& instance : arr) { String name = rule.GetName(); - frame.Locals->Set(rule.GetFKVar(), instance); + frame.Locals->Set(rule.GetFKVar(), instance, true); name += instance; if (EvaluateApplyRuleInstance(checkable, name, frame, rule, skipFilter)) @@ -113,8 +117,8 @@ bool ScheduledDowntime::EvaluateApplyRule(const Checkable::Ptr& checkable, const ObjectLock olock (dict); for (auto& kv : dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + frame.Locals->Set(rule.GetFKVar(), kv.first, true); + frame.Locals->Set(rule.GetFVVar(), kv.second, true); if (EvaluateApplyRuleInstance(checkable, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; diff --git a/lib/icinga/service-apply.cpp b/lib/icinga/service-apply.cpp index ab472a47a..2de40e14c 100644 --- a/lib/icinga/service-apply.cpp +++ b/lib/icinga/service-apply.cpp @@ -61,10 +61,20 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bo CONTEXT("Evaluating 'apply' rule (" << di << ")"); - ScriptFrame frame(true); - if (rule.GetScope()) - rule.GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); + ScriptFrame frame (false); + + if (rule.GetScope() || rule.GetFTerm()) { + frame.Locals = new Dictionary(); + + if (rule.GetScope()) { + rule.GetScope()->CopyTo(frame.Locals); + } + + host->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = host->GetFrozenLocalsForApply(); + } bool match = false; @@ -89,7 +99,7 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bo String name = rule.GetName(); if (!rule.GetFKVar().IsEmpty()) { - frame.Locals->Set(rule.GetFKVar(), instance); + frame.Locals->Set(rule.GetFKVar(), instance, true); name += instance; } @@ -104,8 +114,8 @@ bool Service::EvaluateApplyRule(const Host::Ptr& host, const ApplyRule& rule, bo ObjectLock olock (dict); for (auto& kv : dict) { - frame.Locals->Set(rule.GetFKVar(), kv.first); - frame.Locals->Set(rule.GetFVVar(), kv.second); + frame.Locals->Set(rule.GetFKVar(), kv.first, true); + frame.Locals->Set(rule.GetFVVar(), kv.second, true); if (EvaluateApplyRuleInstance(host, rule.GetName() + kv.first, frame, rule, skipFilter)) match = true; diff --git a/lib/icinga/service.cpp b/lib/icinga/service.cpp index acc6c89e1..4cc8eeef8 100644 --- a/lib/icinga/service.cpp +++ b/lib/icinga/service.cpp @@ -260,3 +260,11 @@ std::pair icinga::GetHostService(const Checkable::Ptr& else return std::make_pair(static_pointer_cast(checkable), nullptr); } + +Dictionary::Ptr Service::MakeLocalsForApply() +{ + return new Dictionary({ + { "host", m_Host }, + { "service", this } + }); +} diff --git a/lib/icinga/service.hpp b/lib/icinga/service.hpp index 558f73c03..a9827a3e4 100644 --- a/lib/icinga/service.hpp +++ b/lib/icinga/service.hpp @@ -48,6 +48,8 @@ public: protected: void CreateChildObjects(const Type::Ptr& childType) override; + Dictionary::Ptr MakeLocalsForApply() override; + private: Host::Ptr m_Host; From 477d177b5a6b1fb0b8bbf97a829286b3b48ecf02 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 15 Nov 2022 12:51:13 +0100 Subject: [PATCH 4/4] Cache locals of script frame used for group assign where eval to avoid malloc(). This BREAKS assign where locals.x = 1. --- lib/icinga/hostgroup.cpp | 18 ++++++++++++++---- lib/icinga/servicegroup.cpp | 19 +++++++++++++------ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/icinga/hostgroup.cpp b/lib/icinga/hostgroup.cpp index 9ab338300..d1d88dac0 100644 --- a/lib/icinga/hostgroup.cpp +++ b/lib/icinga/hostgroup.cpp @@ -24,10 +24,20 @@ bool HostGroup::EvaluateObjectRule(const Host::Ptr& host, const ConfigItem::Ptr& CONTEXT("Evaluating rule for group '" << groupName << "'"); - ScriptFrame frame(true); - if (group->GetScope()) - group->GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); + ScriptFrame frame (false); + + if (group->GetScope()) { + frame.Locals = new Dictionary(); + + if (group->GetScope()) { + group->GetScope()->CopyTo(frame.Locals); + } + + host->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = host->GetFrozenLocalsForApply(); + } if (!group->GetFilter()->Evaluate(frame).GetValue().ToBool()) return false; diff --git a/lib/icinga/servicegroup.cpp b/lib/icinga/servicegroup.cpp index ee2bc9c6e..f3c689446 100644 --- a/lib/icinga/servicegroup.cpp +++ b/lib/icinga/servicegroup.cpp @@ -24,13 +24,20 @@ bool ServiceGroup::EvaluateObjectRule(const Service::Ptr& service, const ConfigI CONTEXT("Evaluating rule for group '" << groupName << "'"); - Host::Ptr host = service->GetHost(); + ScriptFrame frame (false); - ScriptFrame frame(true); - if (group->GetScope()) - group->GetScope()->CopyTo(frame.Locals); - frame.Locals->Set("host", host); - frame.Locals->Set("service", service); + if (group->GetScope()) { + frame.Locals = new Dictionary(); + + if (group->GetScope()) { + group->GetScope()->CopyTo(frame.Locals); + } + + service->GetFrozenLocalsForApply()->CopyTo(frame.Locals); + frame.Locals->Freeze(); + } else { + frame.Locals = service->GetFrozenLocalsForApply(); + } if (!group->GetFilter()->Evaluate(frame).GetValue().ToBool()) return false;