From 972662081d2698961acfb833bf6ae2755d91f1b9 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 | 203 +++++++++++++++----------------------- 1 file changed, 81 insertions(+), 122 deletions(-) diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index 9dc0f1aa25d..b8dea9c20ce 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -450,74 +450,55 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committing " << total << " new items."; #endif /* I2_DEBUG */ - std::set types; - std::set completed_types; + std::vector types; int itemsCount {0}; for (const Type::Ptr& type : Type::GetAllTypes()) { if (ConfigObject::TypeInstance->IsAssignableFrom(type)) - types.insert(type); + types.emplace_back(type); } - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; - - bool unresolved_dep = false; - - /* 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; - } - } + types = Type::SortByLoadDependencies(types); - if (unresolved_dep) - continue; - - std::atomic committed_items(0); - std::mutex newItemsMutex; + for (auto& type : types) { + std::atomic committed_items(0); + std::mutex newItemsMutex; - { - auto items (itemsByType.find(type.get())); + { + auto items (itemsByType.find(type.get())); - if (items != itemsByType.end()) { - upq.ParallelFor(items->second, [&committed_items, &newItems, &newItemsMutex](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + 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(); - } - - return; + if (!item->Commit(ip.second)) { + if (item->IsIgnoreOnError()) { + item->Unregister(); } - committed_items++; + return; + } - std::unique_lock lock(newItemsMutex); - newItems.emplace_back(item); - }); + committed_items++; - upq.Join(); - } - } + std::unique_lock lock(newItemsMutex); + newItems.emplace_back(item); + }); - itemsCount += committed_items; + upq.Join(); + } + } - 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 @@ -525,105 +506,83 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committed " << itemsCount << " items."; #endif /* I2_DEBUG */ - completed_types.clear(); - - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; - - bool unresolved_dep = false; - - /* 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 (unresolved_dep) - continue; - - std::atomic notified_items(0); + for (const Type::Ptr& type : types) { + std::atomic notified_items(0); - { - auto items (itemsByType.find(type.get())); + { + 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 (items != itemsByType.end()) { + upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - if (!item->m_Object) - return; + 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;