Skip to content

Commit 9726620

Browse files
Al2KlimovAl2Klimov
committed
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.
1 parent a67281f commit 9726620

File tree

1 file changed

+81
-122
lines changed

1 file changed

+81
-122
lines changed

‎lib/config/configitem.cpp

+81-122
Original file line numberDiff line numberDiff line change
@@ -450,180 +450,139 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
450450
<< "Committing " << total << " new items.";
451451
#endif /* I2_DEBUG */
452452

453-
std::set<Type::Ptr> types;
454-
std::set<Type::Ptr> completed_types;
453+
std::vector<Type::Ptr> types;
455454
int itemsCount {0};
456455

457456
for (const Type::Ptr& type : Type::GetAllTypes()) {
458457
if (ConfigObject::TypeInstance->IsAssignableFrom(type))
459-
types.insert(type);
458+
types.emplace_back(type);
460459
}
461460

462-
while (types.size() != completed_types.size()) {
463-
for (const Type::Ptr& type : types) {
464-
if (completed_types.find(type) != completed_types.end())
465-
continue;
466-
467-
bool unresolved_dep = false;
468-
469-
/* skip this type (for now) if there are unresolved load dependencies */
470-
for (auto pLoadDep : type->GetLoadDependencies()) {
471-
if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) {
472-
unresolved_dep = true;
473-
break;
474-
}
475-
}
461+
types = Type::SortByLoadDependencies(types);
476462

477-
if (unresolved_dep)
478-
continue;
479-
480-
std::atomic<int> committed_items(0);
481-
std::mutex newItemsMutex;
463+
for (auto& type : types) {
464+
std::atomic<int> committed_items(0);
465+
std::mutex newItemsMutex;
482466

483-
{
484-
auto items (itemsByType.find(type.get()));
467+
{
468+
auto items (itemsByType.find(type.get()));
485469

486-
if (items != itemsByType.end()) {
487-
upq.ParallelFor(items->second, [&committed_items, &newItems, &newItemsMutex](const ItemPair& ip) {
488-
const ConfigItem::Ptr& item = ip.first;
470+
if (items != itemsByType.end()) {
471+
upq.ParallelFor(items->second, [&committed_items, &newItems, &newItemsMutex](const ItemPair& ip) {
472+
const ConfigItem::Ptr& item = ip.first;
489473

490-
if (!item->Commit(ip.second)) {
491-
if (item->IsIgnoreOnError()) {
492-
item->Unregister();
493-
}
494-
495-
return;
474+
if (!item->Commit(ip.second)) {
475+
if (item->IsIgnoreOnError()) {
476+
item->Unregister();
496477
}
497478

498-
committed_items++;
479+
return;
480+
}
499481

500-
std::unique_lock<std::mutex> lock(newItemsMutex);
501-
newItems.emplace_back(item);
502-
});
482+
committed_items++;
503483

504-
upq.Join();
505-
}
506-
}
484+
std::unique_lock<std::mutex> lock(newItemsMutex);
485+
newItems.emplace_back(item);
486+
});
507487

508-
itemsCount += committed_items;
488+
upq.Join();
489+
}
490+
}
509491

510-
completed_types.insert(type);
492+
itemsCount += committed_items;
511493

512494
#ifdef I2_DEBUG
513-
if (committed_items > 0)
514-
Log(LogDebug, "configitem")
515-
<< "Committed " << committed_items << " items of type '" << type->GetName() << "'.";
495+
if (committed_items > 0)
496+
Log(LogDebug, "configitem")
497+
<< "Committed " << committed_items << " items of type '" << type->GetName() << "'.";
516498
#endif /* I2_DEBUG */
517499

518-
if (upq.HasExceptions())
519-
return false;
520-
}
500+
if (upq.HasExceptions())
501+
return false;
521502
}
522503

523504
#ifdef I2_DEBUG
524505
Log(LogDebug, "configitem")
525506
<< "Committed " << itemsCount << " items.";
526507
#endif /* I2_DEBUG */
527508

528-
completed_types.clear();
529-
530-
while (types.size() != completed_types.size()) {
531-
for (const Type::Ptr& type : types) {
532-
if (completed_types.find(type) != completed_types.end())
533-
continue;
534-
535-
bool unresolved_dep = false;
536-
537-
/* skip this type (for now) if there are unresolved load dependencies */
538-
for (auto pLoadDep : type->GetLoadDependencies()) {
539-
if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) {
540-
unresolved_dep = true;
541-
break;
542-
}
543-
}
544-
545-
if (unresolved_dep)
546-
continue;
547-
548-
std::atomic<int> notified_items(0);
509+
for (const Type::Ptr& type : types) {
510+
std::atomic<int> notified_items(0);
549511

550-
{
551-
auto items (itemsByType.find(type.get()));
512+
{
513+
auto items (itemsByType.find(type.get()));
552514

553-
if (items != itemsByType.end()) {
554-
upq.ParallelFor(items->second, [&notified_items](const ItemPair& ip) {
555-
const ConfigItem::Ptr& item = ip.first;
515+
if (items != itemsByType.end()) {
516+
upq.ParallelFor(items->second, [&notified_items](const ItemPair& ip) {
517+
const ConfigItem::Ptr& item = ip.first;
556518

557-
if (!item->m_Object)
558-
return;
519+
if (!item->m_Object)
520+
return;
559521

560-
try {
561-
item->m_Object->OnAllConfigLoaded();
562-
notified_items++;
563-
} catch (const std::exception& ex) {
564-
if (!item->m_IgnoreOnError)
565-
throw;
522+
try {
523+
item->m_Object->OnAllConfigLoaded();
524+
notified_items++;
525+
} catch (const std::exception& ex) {
526+
if (!item->m_IgnoreOnError)
527+
throw;
566528

567-
Log(LogNotice, "ConfigObject")
568-
<< "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex);
529+
Log(LogNotice, "ConfigObject")
530+
<< "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex);
569531

570-
item->Unregister();
532+
item->Unregister();
571533

572-
{
573-
std::unique_lock<std::mutex> lock(item->m_Mutex);
574-
item->m_IgnoredItems.push_back(item->m_DebugInfo.Path);
575-
}
534+
{
535+
std::unique_lock<std::mutex> lock(item->m_Mutex);
536+
item->m_IgnoredItems.push_back(item->m_DebugInfo.Path);
576537
}
577-
});
538+
}
539+
});
578540

579-
upq.Join();
580-
}
541+
upq.Join();
581542
}
582-
583-
completed_types.insert(type);
543+
}
584544

585545
#ifdef I2_DEBUG
586-
if (notified_items > 0)
587-
Log(LogDebug, "configitem")
588-
<< "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'.";
546+
if (notified_items > 0)
547+
Log(LogDebug, "configitem")
548+
<< "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'.";
589549
#endif /* I2_DEBUG */
590550

591-
if (upq.HasExceptions())
592-
return false;
551+
if (upq.HasExceptions())
552+
return false;
593553

594-
notified_items = 0;
595-
for (auto loadDep : type->GetLoadDependencies()) {
596-
auto items (itemsByType.find(loadDep));
554+
notified_items = 0;
555+
for (auto loadDep : type->GetLoadDependencies()) {
556+
auto items (itemsByType.find(loadDep));
597557

598-
if (items != itemsByType.end()) {
599-
upq.ParallelFor(items->second, [&type, &notified_items](const ItemPair& ip) {
600-
const ConfigItem::Ptr& item = ip.first;
558+
if (items != itemsByType.end()) {
559+
upq.ParallelFor(items->second, [&type, &notified_items](const ItemPair& ip) {
560+
const ConfigItem::Ptr& item = ip.first;
601561

602-
if (!item->m_Object)
603-
return;
562+
if (!item->m_Object)
563+
return;
604564

605-
ActivationScope ascope(item->m_ActivationContext);
606-
item->m_Object->CreateChildObjects(type);
607-
notified_items++;
608-
});
609-
}
565+
ActivationScope ascope(item->m_ActivationContext);
566+
item->m_Object->CreateChildObjects(type);
567+
notified_items++;
568+
});
610569
}
570+
}
611571

612-
upq.Join();
572+
upq.Join();
613573

614574
#ifdef I2_DEBUG
615-
if (notified_items > 0)
616-
Log(LogDebug, "configitem")
617-
<< "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'.";
575+
if (notified_items > 0)
576+
Log(LogDebug, "configitem")
577+
<< "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'.";
618578
#endif /* I2_DEBUG */
619579

620-
if (upq.HasExceptions())
621-
return false;
580+
if (upq.HasExceptions())
581+
return false;
622582

623-
// Make sure to activate any additionally generated items
624-
if (!CommitNewItems(context, upq, newItems))
625-
return false;
626-
}
583+
// Make sure to activate any additionally generated items
584+
if (!CommitNewItems(context, upq, newItems))
585+
return false;
627586
}
628587

629588
return true;

0 commit comments

Comments
 (0)