[Rework] Dependencies must be unique...

diff --git a/src/libserver/symcache/symcache_impl.cxx b/src/libserver/symcache/symcache_impl.cxx
index 34d399c5dbd903637cfd2571c52c87b880e89f00..3fe2eb580b6926ced0146fc45178a5830e5afdef 100644 (file)
--- 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<std::pair<double, const cache_item *>
                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 ca81267c489613c2426e20a1114dddf5ed1800cd..9bfd83681a8ce2059318b711788f74b84dae6804 100644 (file)
--- 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 95127f850e9753fa7bde83e9fdba54cf85f0f794..acbcf68ffe6875cbfce3b776ff290e5ddc4c28fc 100644 (file)
--- 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<cache_item> {
                augmentations;
 
        /* Dependencies */
-       std::vector<cache_dependency> deps;
+       ankerl::unordered_dense::map<int, cache_dependency> deps;
        /* Reverse dependencies */
-       std::vector<cache_dependency> rdeps;
+       ankerl::unordered_dense::map<int, cache_dependency> rdeps;
 
 public:
        /**

diff --git a/src/libserver/symcache/symcache_runtime.cxx b/src/libserver/symcache/symcache_runtime.cxx
index 8c2df4696b34e201f0f9cb22b2dcb97664c33043..bbf3b3d0079951b7dadeb9f67c5b76c91aff5b24 100644 (file)
--- 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) {