ConfigItem::CommitNewItems(): pre-sort types by their load dependencies once

[Al2Klimov]

to avoid complicated nested loops, iterating over the same types and
checking dependencies over and over, skipping already completed ones.

[yhabteab]

Instead of copying the types around, you can access Type::GetAllTypes() directly within your new method or pass the types without having to copy them if you need the arguments for your unittest. You can then perform this check below when they are actually loaded.

icinga2/lib/config/configitem.cpp

Lines 456 to 459 in 9726620

	for (const Type::Ptr& type : Type::GetAllTypes()) {
	if (ConfigObject::TypeInstance->IsAssignableFrom(type))
	types.emplace_back(type);
	}

icinga2/lib/base/type.cpp

Lines 85 to 87 in 9726620

	for (auto& type : types) {
	pending.emplace(type.get());
	}

[Al2Klimov]

Yes, we should address such in

• Avoid runtime-checked cast from Type::Ptr to ConfigType* in IcingaDB::UpdateAllConfigObjects #8753

one nice day.

[yhabteab]

I wasn't suggesting you to remove this check, I was just pointing out that you are unnecessarily copying around the types and should avoid doing that. It's not something that already existed and simply saying it will be fixed "one nice day" isn't an argument.

[Al2Klimov]

But not all types are config types.

[yhabteab]

But not all types are config types.

Why would that matter to you?

[Al2Klimov]

CONFIG items get committed here, of course only CONFIG types are relevant here. Btw. that preserved check IS "something that already existed" (just like #10003 (comment)).

[yhabteab]

I don't think you get my point her! My suggestion is to drop this loop entirely and move the check down here instead. Your sort method shouldn't care whether a type is assignable to a config object or not.

[yhabteab]

You've omitted a check from the previous version of the code.

types.find(pLoadDep) != types.end() ---

[Al2Klimov]

Type::SortByLoadDependencies() auto-includes all load dependencies, so this case vanishes.

[julianbrost]

Sort functions do not add values, so if it does, the function name is misleading.

[Al2Klimov]

Re-added it.