From a422419c5b378f64a1a0bc826d6a268147c22cb7 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 15 Feb 2024 11:46:27 +0100 Subject: [PATCH] ConfigItem::CommitNewItems(): pre-sort types by their load dependencies once to avoid complicated nested loops, iterating over the same types and checking dependencies over and over, skipping already completed ones. --- lib/config/configitem.cpp | 201 ++++++++++++++------------------------ 1 file changed, 76 insertions(+), 125 deletions(-) diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index bf4da81a4be..6e7bd8ffbfe 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -444,74 +444,47 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committing " << total << " new items."; #endif /* I2_DEBUG */ - std::set types; - std::set completed_types; int itemsCount {0}; - for (const Type::Ptr& type : Type::GetAllTypes()) { - if (ConfigObject::TypeInstance->IsAssignableFrom(type)) - types.insert(type); - } - - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; + for (auto& type : Type::GetAllConfigTypesSortedByLoadDependencies()) { + std::atomic committed_items(0); - bool unresolved_dep = false; + { + auto items (itemsByType.find(type.get())); - /* skip this type (for now) if there are unresolved load dependencies */ - for (auto pLoadDep : type->GetLoadDependencies()) { - if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { - unresolved_dep = true; - break; + if (items != itemsByType.end()) { + for (const ItemPair& pair: items->second) { + newItems.emplace_back(pair.first); } - } - - if (unresolved_dep) - continue; - std::atomic committed_items(0); + upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - { - auto items (itemsByType.find(type.get())); + if (!item->Commit(ip.second)) { + if (item->IsIgnoreOnError()) { + item->Unregister(); + } - if (items != itemsByType.end()) { - for (const ItemPair& pair: items->second) { - newItems.emplace_back(pair.first); + return; } - upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + committed_items++; + }); - if (!item->Commit(ip.second)) { - if (item->IsIgnoreOnError()) { - item->Unregister(); - } - - return; - } - - committed_items++; - }); - - upq.Join(); - } + upq.Join(); } + } - itemsCount += committed_items; - - completed_types.insert(type); + itemsCount += committed_items; #ifdef I2_DEBUG - if (committed_items > 0) - Log(LogDebug, "configitem") - << "Committed " << committed_items << " items of type '" << type->GetName() << "'."; + if (committed_items > 0) + Log(LogDebug, "configitem") + << "Committed " << committed_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; - } + if (upq.HasExceptions()) + return false; } #ifdef I2_DEBUG @@ -519,105 +492,83 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committed " << itemsCount << " items."; #endif /* I2_DEBUG */ - completed_types.clear(); + for (auto& type : Type::GetAllConfigTypesSortedByLoadDependencies()) { + std::atomic notified_items(0); - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; + { + auto items (itemsByType.find(type.get())); - bool unresolved_dep = false; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - /* skip this type (for now) if there are unresolved load dependencies */ - for (auto pLoadDep : type->GetLoadDependencies()) { - if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { - unresolved_dep = true; - break; - } - } + if (!item->m_Object) + return; - if (unresolved_dep) - continue; - - std::atomic notified_items(0); - - { - auto items (itemsByType.find(type.get())); - - if (items != itemsByType.end()) { - upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; - - if (!item->m_Object) - return; + try { + item->m_Object->OnAllConfigLoaded(); + notified_items++; + } catch (const std::exception& ex) { + if (!item->m_IgnoreOnError) + throw; - try { - item->m_Object->OnAllConfigLoaded(); - notified_items++; - } catch (const std::exception& ex) { - if (!item->m_IgnoreOnError) - throw; + Log(LogNotice, "ConfigObject") + << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); - Log(LogNotice, "ConfigObject") - << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); + item->Unregister(); - item->Unregister(); - - { - std::unique_lock lock(item->m_Mutex); - item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); - } + { + std::unique_lock lock(item->m_Mutex); + item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); } - }); + } + }); - upq.Join(); - } + upq.Join(); } - - completed_types.insert(type); + } #ifdef I2_DEBUG - if (notified_items > 0) - Log(LogDebug, "configitem") - << "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'."; + if (notified_items > 0) + Log(LogDebug, "configitem") + << "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; + if (upq.HasExceptions()) + return false; - notified_items = 0; - for (auto loadDep : type->GetLoadDependencies()) { - auto items (itemsByType.find(loadDep)); + notified_items = 0; + for (auto loadDep : type->GetLoadDependencies()) { + auto items (itemsByType.find(loadDep)); - if (items != itemsByType.end()) { - upq.ParallelFor(items->second, [&type, ¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [&type, ¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - if (!item->m_Object) - return; + if (!item->m_Object) + return; - ActivationScope ascope(item->m_ActivationContext); - item->m_Object->CreateChildObjects(type); - notified_items++; - }); - } + ActivationScope ascope(item->m_ActivationContext); + item->m_Object->CreateChildObjects(type); + notified_items++; + }); } + } - upq.Join(); + upq.Join(); #ifdef I2_DEBUG - if (notified_items > 0) - Log(LogDebug, "configitem") - << "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'."; + if (notified_items > 0) + Log(LogDebug, "configitem") + << "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; + if (upq.HasExceptions()) + return false; - // Make sure to activate any additionally generated items - if (!CommitNewItems(context, upq, newItems)) - return false; - } + // Make sure to activate any additionally generated items + if (!CommitNewItems(context, upq, newItems)) + return false; } return true;