From 34d714090ab9f06ed54a9b59da47012186ab0537 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Tue, 18 Jun 2024 15:19:41 +0100 Subject: [PATCH] [Rework] Dependencies must be unique... --- src/libserver/symcache/symcache_impl.cxx | 38 +++++++++++----- src/libserver/symcache/symcache_item.cxx | 48 ++++++++++++++++----- src/libserver/symcache/symcache_item.hxx | 4 +- src/libserver/symcache/symcache_runtime.cxx | 4 +- 4 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/libserver/symcache/symcache_impl.cxx b/src/libserver/symcache/symcache_impl.cxx index 34d399c5d..3fe2eb580 100644 --- a/src/libserver/symcache/symcache_impl.cxx +++ b/src/libserver/symcache/symcache_impl.cxx @@ -535,10 +535,19 @@ auto symcache::add_dependency(int id_from, std::string_view to, int virtual_id_f const auto &source = items_by_id[id_from]; g_assert(source.get() != nullptr); - source->deps.emplace_back(nullptr, - std::string(to), - id_from, - -1); + if (!source->deps.contains(id_from)) { + msg_debug_cache("add dependency %s -> %s", + source->symbol.c_str(), to.data()); + source->deps.emplace(id_from, cache_dependency{nullptr, + std::string(to), + id_from, + -1}); + } + else { + msg_debug_cache("duplicate dependency %s -> %s", + source->symbol.c_str(), to.data()); + return; + } if (virtual_id_from >= 0) { @@ -546,10 +555,19 @@ auto symcache::add_dependency(int id_from, std::string_view to, int virtual_id_f /* We need that for settings id propagation */ const auto &vsource = items_by_id[virtual_id_from]; g_assert(vsource.get() != nullptr); - vsource->deps.emplace_back(nullptr, - std::string(to), - -1, - virtual_id_from); + + if (!vsource->deps.contains(virtual_id_from)) { + msg_debug_cache("add virtual dependency %s -> %s", + vsource->symbol.c_str(), to.data()); + vsource->deps.emplace(virtual_id_from, cache_dependency{nullptr, + std::string(to), + -1, + virtual_id_from}); + } + else { + msg_debug_cache("duplicate virtual dependency %s -> %s", + vsource->symbol.c_str(), to.data()); + } } } @@ -625,7 +643,7 @@ auto symcache::resort() -> void tsort_mark(it, tsort_mask::TEMP); msg_debug_cache_lambda("visiting node: %s (%d)", it->symbol.c_str(), cur_order); - for (const auto &dep: it->deps) { + for (const auto &[id, dep]: it->deps) { msg_debug_cache_lambda("visiting dep: %s (%d)", dep.item->symbol.c_str(), cur_order + 1); rec(dep.item, cur_order + 1, rec); } @@ -1217,7 +1235,7 @@ auto symcache::get_max_timeout(std::vector auto own_timeout = get_item_timeout(it); auto max_child_timeout = 0.0; - for (const auto &dep: it->deps) { + for (const auto &[id, dep]: it->deps) { auto cld_timeout = self(dep.item, self); if (cld_timeout > max_child_timeout) { diff --git a/src/libserver/symcache/symcache_item.cxx b/src/libserver/symcache/symcache_item.cxx index ca81267c4..9bfd83681 100644 --- a/src/libserver/symcache/symcache_item.cxx +++ b/src/libserver/symcache/symcache_item.cxx @@ -78,7 +78,7 @@ auto cache_item::process_deps(const symcache &cache) -> void /* Allow logging macros to work */ auto log_tag = [&]() { return cache.log_tag(); }; - for (auto &dep: deps) { + for (auto &[_id, dep]: deps) { msg_debug_cache("process real dependency %s on %s", symbol.c_str(), dep.sym.c_str()); auto *dit = cache.get_item_by_name_mut(dep.sym, true); @@ -148,6 +148,8 @@ auto cache_item::process_deps(const symcache &cache) -> void continue; } + + dep.item = dit; } else { if (dit->id == id) { @@ -161,20 +163,35 @@ auto cache_item::process_deps(const symcache &cache) -> void auto *parent = get_parent_mut(cache); if (parent) { - dit->rdeps.emplace_back(parent, parent->symbol, parent->id, -1); + if (!dit->rdeps.contains(parent->id)) { + dit->rdeps.emplace(parent->id, cache_dependency{parent, parent->symbol, parent->id, -1}); + msg_debug_cache("added reverse dependency from %d on %d", parent->id, + dit->id); + } + else { + msg_debug_cache("reverse dependency from %d on %d already exists", + parent->id, dit->id); + } dep.item = dit; dep.id = dit->id; - - msg_debug_cache("added reverse dependency from %d on %d", parent->id, - dit->id); + } + else { + msg_err_cache("cannot find parent for virtual symbol %s, when resolving dependency %s", + symbol.c_str(), dep.sym.c_str()); } } else { dep.item = dit; dep.id = dit->id; - dit->rdeps.emplace_back(this, symbol, id, -1); - msg_debug_cache("added reverse dependency from %d on %d", id, - dit->id); + if (!dit->rdeps.contains(id)) { + dit->rdeps.emplace(id, cache_dependency{this, symbol, id, -1}); + msg_debug_cache("added reverse dependency from %d on %d", id, + dit->id); + } + else { + msg_debug_cache("reverse dependency from %d on %d already exists", + id, dit->id); + } } } } @@ -185,12 +202,21 @@ auto cache_item::process_deps(const symcache &cache) -> void continue; } + else { + msg_err_cache("cannot find dependency named %s for symbol %s", + dep.sym.c_str(), symbol.c_str()); + } } // Remove empty deps - deps.erase(std::remove_if(std::begin(deps), std::end(deps), - [](const auto &dep) { return !dep.item; }), - std::end(deps)); + for (auto it = deps.begin(); it != deps.end();) { + if (it->second.item == nullptr) { + it = deps.erase(it); + } + else { + ++it; + } + } } auto cache_item::resolve_parent(const symcache &cache) -> bool diff --git a/src/libserver/symcache/symcache_item.hxx b/src/libserver/symcache/symcache_item.hxx index 95127f850..acbcf68ff 100644 --- a/src/libserver/symcache/symcache_item.hxx +++ b/src/libserver/symcache/symcache_item.hxx @@ -246,9 +246,9 @@ struct cache_item : std::enable_shared_from_this { augmentations; /* Dependencies */ - std::vector deps; + ankerl::unordered_dense::map deps; /* Reverse dependencies */ - std::vector rdeps; + ankerl::unordered_dense::map rdeps; public: /** diff --git a/src/libserver/symcache/symcache_runtime.cxx b/src/libserver/symcache/symcache_runtime.cxx index 8c2df4696..bbf3b3d00 100644 --- a/src/libserver/symcache/symcache_runtime.cxx +++ b/src/libserver/symcache/symcache_runtime.cxx @@ -588,7 +588,7 @@ auto symcache_runtime::check_item_deps(struct rspamd_task *task, symcache &cache auto ret = true; - for (const auto &dep: item->deps) { + for (const auto &[id, dep]: item->deps) { if (!dep.item) { /* Assume invalid deps as done */ msg_debug_cache_task_lambda("symbol %d(%s) has invalid dependencies on %d(%s)", @@ -814,7 +814,7 @@ auto symcache_runtime::process_item_rdeps(struct rspamd_task *task, cache_item * return; } - for (const auto &rdep: item->rdeps) { + for (const auto &[id, rdep]: item->rdeps.values()) { if (rdep.item) { auto *dyn_item = get_dynamic_item(rdep.item->id); if (dyn_item->status == cache_item_status::not_started) { -- 2.39.5