From 33e609d7919b79fb2ae7877ab367389045b4dec9 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Fri, 28 Oct 2022 13:11:58 +0200 Subject: [PATCH 1/2] Type#GetLoadDependencies(): avoid malloc() - cache result - return it by const ref - do Type::GetByName() for the callers --- lib/base/type.cpp | 5 +++-- lib/base/type.hpp | 3 ++- lib/config/configitem.cpp | 10 ++++------ tools/mkclass/classcompiler.cpp | 10 ++++++---- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/base/type.cpp b/lib/base/type.cpp index 493833d0c..6e4fd6b57 100644 --- a/lib/base/type.cpp +++ b/lib/base/type.cpp @@ -136,9 +136,10 @@ Value Type::GetField(int id) const BOOST_THROW_EXCEPTION(std::runtime_error("Invalid field ID.")); } -std::vector Type::GetLoadDependencies() const +const std::unordered_set& Type::GetLoadDependencies() const { - return std::vector(); + static const std::unordered_set noDeps; + return noDeps; } int Type::GetActivationPriority() const diff --git a/lib/base/type.hpp b/lib/base/type.hpp index 2bf54ccf5..f47299560 100644 --- a/lib/base/type.hpp +++ b/lib/base/type.hpp @@ -7,6 +7,7 @@ #include "base/string.hpp" #include "base/object.hpp" #include "base/initialize.hpp" +#include #include namespace icinga @@ -85,7 +86,7 @@ public: void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty) override; Value GetField(int id) const override; - virtual std::vector GetLoadDependencies() const; + virtual const std::unordered_set& GetLoadDependencies() const; virtual int GetActivationPriority() const; typedef std::function AttributeHandler; diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index d61509457..cd23cd858 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -450,8 +450,7 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue bool unresolved_dep = false; /* skip this type (for now) if there are unresolved load dependencies */ - for (const String& loadDep : type->GetLoadDependencies()) { - Type::Ptr pLoadDep = Type::GetByName(loadDep); + for (auto pLoadDep : type->GetLoadDependencies()) { if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { unresolved_dep = true; break; @@ -516,8 +515,7 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue bool unresolved_dep = false; /* skip this type (for now) if there are unresolved load dependencies */ - for (const String& loadDep : type->GetLoadDependencies()) { - Type::Ptr pLoadDep = Type::GetByName(loadDep); + for (auto pLoadDep : type->GetLoadDependencies()) { if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { unresolved_dep = true; break; @@ -567,11 +565,11 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue return false; notified_items = 0; - for (const String& loadDep : type->GetLoadDependencies()) { + for (auto loadDep : type->GetLoadDependencies()) { upq.ParallelFor(items, [loadDep, &type, ¬ified_items](const ItemPair& ip) { const ConfigItem::Ptr& item = ip.first; - if (!item->m_Object || item->m_Type->GetName() != loadDep) + if (!item->m_Object || item->m_Type.get() != loadDep) return; ActivationScope ascope(item->m_ActivationContext); diff --git a/tools/mkclass/classcompiler.cpp b/tools/mkclass/classcompiler.cpp index 8df23486d..3a49576b8 100644 --- a/tools/mkclass/classcompiler.cpp +++ b/tools/mkclass/classcompiler.cpp @@ -376,14 +376,16 @@ void ClassCompiler::HandleClass(const Klass& klass, const ClassDebugInfo&) << "}" << std::endl << std::endl; /* GetLoadDependencies */ - m_Header << "\t" << "std::vector GetLoadDependencies() const override;" << std::endl; + m_Header << "\t" << "const std::unordered_set& GetLoadDependencies() const override;" << std::endl; - m_Impl << "std::vector TypeImpl<" << klass.Name << ">::GetLoadDependencies() const" << std::endl + m_Impl << "const std::unordered_set& TypeImpl<" << klass.Name << ">::GetLoadDependencies() const" << std::endl << "{" << std::endl - << "\t" << "std::vector deps;" << std::endl; + << "\t" << "static const std::unordered_set deps ({" << std::endl; for (const std::string& dep : klass.LoadDependencies) - m_Impl << "\t" << "deps.emplace_back(\"" << dep << "\");" << std::endl; + m_Impl << "\t\t" << "GetByName(\"" << dep << "\").get()," << std::endl; + + m_Impl << "\t" << "});" << std::endl; m_Impl << "\t" << "return deps;" << std::endl << "}" << std::endl << std::endl; From ae693cb7e1df1b885142854cf8a0f8a7600a3fb7 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 9 Nov 2022 15:08:43 +0100 Subject: [PATCH 2/2] ConfigItem::CommitNewItems(): allow fast search of pending items by type --- lib/config/configitem.cpp | 126 +++++++++++++++++++++++--------------- 1 file changed, 76 insertions(+), 50 deletions(-) diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index cd23cd858..8beaa4463 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -26,6 +26,7 @@ #include #include #include +#include using namespace icinga; @@ -387,12 +388,15 @@ ConfigItem::Ptr ConfigItem::GetByTypeAndName(const Type::Ptr& type, const String bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue& upq, std::vector& newItems) { typedef std::pair ItemPair; - std::vector items; + std::unordered_map> itemsByType; + std::vector::size_type total = 0; { std::unique_lock lock(m_Mutex); for (const TypeMap::value_type& kv : m_Items) { + std::vector items; + for (const ItemMap::value_type& kv2 : kv.second) { if (kv2.second->m_Abstract || kv2.second->m_Object) continue; @@ -402,6 +406,11 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue items.emplace_back(kv2.second, false); } + + if (!items.empty()) { + total += items.size(); + itemsByType.emplace(kv.first.get(), std::move(items)); + } } ItemList newUnnamedItems; @@ -415,22 +424,25 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue if (item->m_Abstract || item->m_Object) continue; - items.emplace_back(item, true); + itemsByType[item->m_Type.get()].emplace_back(item, true); + ++total; } m_UnnamedItems.swap(newUnnamedItems); } - if (items.empty()) + if (!total) return true; // Shuffle all items to evenly distribute them over the threads of the workqueue. This increases perfomance // noticably in environments with lots of objects and available threads. - std::shuffle(std::begin(items), std::end(items), std::default_random_engine {}); + for (auto& kv : itemsByType) { + std::shuffle(std::begin(kv.second), std::end(kv.second), std::default_random_engine{}); + } #ifdef I2_DEBUG Log(LogDebug, "configitem") - << "Committing " << items.size() << " new items."; + << "Committing " << total << " new items."; #endif /* I2_DEBUG */ std::set types; @@ -463,27 +475,30 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue std::atomic committed_items(0); std::mutex newItemsMutex; - upq.ParallelFor(items, [&type, &committed_items, &newItems, &newItemsMutex](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + { + auto items (itemsByType.find(type.get())); - if (item->m_Type != type) - return; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [&committed_items, &newItems, &newItemsMutex](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - if (!item->Commit(ip.second)) { - if (item->IsIgnoreOnError()) { - item->Unregister(); - } + if (!item->Commit(ip.second)) { + if (item->IsIgnoreOnError()) { + item->Unregister(); + } - return; + return; + } + + committed_items++; + + std::unique_lock lock(newItemsMutex); + newItems.emplace_back(item); + }); + + upq.Join(); } - - committed_items++; - - std::unique_lock lock(newItemsMutex); - newItems.emplace_back(item); - }); - - upq.Join(); + } itemsCount += committed_items; @@ -526,35 +541,42 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue continue; std::atomic notified_items(0); - upq.ParallelFor(items, [&type, ¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; - if (!item->m_Object || item->m_Type != type) - return; + { + auto items (itemsByType.find(type.get())); - try { - item->m_Object->OnAllConfigLoaded(); - notified_items++; - } catch (const std::exception& ex) { - if (!item->m_IgnoreOnError) - throw; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - Log(LogNotice, "ConfigObject") - << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); + if (!item->m_Object) + return; - item->Unregister(); + try { + item->m_Object->OnAllConfigLoaded(); + notified_items++; + } catch (const std::exception& ex) { + if (!item->m_IgnoreOnError) + throw; - { - std::unique_lock lock(item->m_Mutex); - item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); - } + Log(LogNotice, "ConfigObject") + << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); + + item->Unregister(); + + { + std::unique_lock lock(item->m_Mutex); + item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); + } + } + }); + + upq.Join(); } - }); + } completed_types.insert(type); - upq.Join(); - #ifdef I2_DEBUG if (notified_items > 0) Log(LogDebug, "configitem") @@ -566,16 +588,20 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue notified_items = 0; for (auto loadDep : type->GetLoadDependencies()) { - upq.ParallelFor(items, [loadDep, &type, ¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + auto items (itemsByType.find(loadDep)); - if (!item->m_Object || item->m_Type.get() != loadDep) - return; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [&type, ¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - ActivationScope ascope(item->m_ActivationContext); - item->m_Object->CreateChildObjects(type); - notified_items++; - }); + if (!item->m_Object) + return; + + ActivationScope ascope(item->m_ActivationContext); + item->m_Object->CreateChildObjects(type); + notified_items++; + }); + } } upq.Join();