From f352f18cfb1448386a9c32a06042f4d94be13b27 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Mon, 9 May 2022 21:32:35 +0100 Subject: [PATCH] [Rework] Use dynamic items in the callbacks --- src/libserver/symcache/symcache_c.cxx | 6 +- src/libserver/symcache/symcache_runtime.cxx | 67 ++++----------------- src/libserver/symcache/symcache_runtime.hxx | 6 +- 3 files changed, 17 insertions(+), 62 deletions(-) diff --git a/src/libserver/symcache/symcache_c.cxx b/src/libserver/symcache/symcache_c.cxx index f8e4389f5..f67078b62 100644 --- a/src/libserver/symcache/symcache_c.cxx +++ b/src/libserver/symcache/symcache_c.cxx @@ -580,7 +580,7 @@ rspamd_symcache_composites_foreach(struct rspamd_task *task, auto *cache_runtime = C_API_SYMCACHE_RUNTIME(task->symcache_runtime); real_cache->composites_foreach([&](const auto *item) { - auto *dyn_item = cache_runtime->get_dynamic_item(item->id, false); + auto *dyn_item = cache_runtime->get_dynamic_item(item->id); if (!dyn_item->started) { func((void *)item->get_name().c_str(), item->get_cbdata(), fd); @@ -609,7 +609,7 @@ rspamd_symcache_finalize_item(struct rspamd_task *task, struct rspamd_symcache_dynamic_item *item) { auto *cache_runtime = C_API_SYMCACHE_RUNTIME(task->symcache_runtime); - auto *real_item = C_API_SYMCACHE_ITEM(item); + auto *real_dyn_item = C_API_SYMCACHE_DYN_ITEM(item); - cache_runtime->finalize_item(task, real_item); + cache_runtime->finalize_item(task, real_dyn_item); } \ No newline at end of file diff --git a/src/libserver/symcache/symcache_runtime.cxx b/src/libserver/symcache/symcache_runtime.cxx index 2d941c0ed..b9706c62a 100644 --- a/src/libserver/symcache/symcache_runtime.cxx +++ b/src/libserver/symcache/symcache_runtime.cxx @@ -47,11 +47,6 @@ symcache_runtime::create(struct rspamd_task *task, symcache &cache) -> symcache_ rspamd_mempool_add_destructor(task->task_pool, symcache_runtime::savepoint_dtor, checkpoint); - for (auto &pair: checkpoint->last_id_mappings) { - pair.first = -1; - pair.second = -1; - } - /* Calculate profile probability */ ev_now_update_if_cheap(task->event_loop); ev_tstamp now = ev_now(task->event_loop); @@ -177,7 +172,7 @@ symcache_runtime::disable_symbol(struct rspamd_task *task, const symcache &cache if (item != nullptr) { - auto *dyn_item = get_dynamic_item(item->id, false); + auto *dyn_item = get_dynamic_item(item->id); if (dyn_item) { dyn_item->finished = true; @@ -204,7 +199,7 @@ symcache_runtime::enable_symbol(struct rspamd_task *task, const symcache &cache, if (item != nullptr) { - auto *dyn_item = get_dynamic_item(item->id, false); + auto *dyn_item = get_dynamic_item(item->id); if (dyn_item) { dyn_item->finished = false; @@ -231,7 +226,7 @@ symcache_runtime::is_symbol_checked(const symcache &cache, std::string_view name if (item != nullptr) { - auto *dyn_item = get_dynamic_item(item->id, true); + auto *dyn_item = get_dynamic_item(item->id); if (dyn_item) { return dyn_item->started; @@ -252,7 +247,7 @@ symcache_runtime::is_symbol_enabled(struct rspamd_task *task, const symcache &ca return false; } else { - auto *dyn_item = get_dynamic_item(item->id, true); + auto *dyn_item = get_dynamic_item(item->id); if (dyn_item) { if (dyn_item->started) { @@ -274,53 +269,14 @@ symcache_runtime::is_symbol_enabled(struct rspamd_task *task, const symcache &ca return true; } -auto symcache_runtime::get_dynamic_item(int id, bool save_in_cache) const -> cache_dynamic_item * +auto symcache_runtime::get_dynamic_item(int id) const -> cache_dynamic_item * { - /* Lookup in cache */ - for (const auto &cache_id: last_id_mappings) { - if (cache_id.first == -1) { - break; - } - if (cache_id.first == id) { - auto *dyn_item = &dynamic_items[cache_id.second]; - - return dyn_item; - } - } /* Not found in the cache, do a hash lookup */ auto our_id_maybe = rspamd::find_map(order->by_cache_id, id); if (our_id_maybe) { - auto *dyn_item = &dynamic_items[our_id_maybe.value()]; - - if (!save_in_cache) { - return dyn_item; - } - - /* Insert in the cache, swapping the first item with the last empty item */ - auto first_known = last_id_mappings[0]; - last_id_mappings[0].first = id; - last_id_mappings[0].second = our_id_maybe.value(); - - if (first_known.first != -1) { - /* This loop is guaranteed to finish as we have just inserted one item */ - - constexpr const auto cache_size = sizeof(last_id_mappings) / sizeof(last_id_mappings[0]); - int i = cache_size - 1; - for (;; --i) { - if (last_id_mappings[i].first != -1) { - if (i < cache_size - 1) { - i++; - } - break; - } - } - - last_id_mappings[i] = first_known; - } - - return dyn_item; + return &dynamic_items[our_id_maybe.value()]; } return nullptr; @@ -363,7 +319,7 @@ symcache_runtime::process_pre_postfilters(struct rspamd_task *task, auto compare_functor = +[](int a, int b) { return a < b; }; auto proc_func = [&](cache_item *item) { - auto dyn_item = get_dynamic_item(item->id, true); + auto dyn_item = get_dynamic_item(item->id); if (!dyn_item->started && !dyn_item->finished) { if (has_slow) { @@ -598,7 +554,7 @@ auto symcache_runtime::check_item_deps(struct rspamd_task *task, symcache &cache continue; } - auto *dep_dyn_item = get_dynamic_item(dep.item->id, true); + auto *dep_dyn_item = get_dynamic_item(dep.item->id); if (!dep_dyn_item->finished) { if (!dep_dyn_item->started) { @@ -703,13 +659,14 @@ rspamd_delayed_timer_dtor(gpointer d) } auto -symcache_runtime::finalize_item(struct rspamd_task *task, cache_item *item) -> void +symcache_runtime::finalize_item(struct rspamd_task *task, cache_dynamic_item *dyn_item) -> void { /* Limit to consider a rule as slow (in milliseconds) */ constexpr const gdouble slow_diff_limit = 300; + auto *item = get_item_by_dynamic_item(dyn_item); /* Sanity checks */ g_assert (items_inflight > 0); - auto *dyn_item = get_dynamic_item(item->id, false); + g_assert (item != nullptr); if (dyn_item->async_events > 0) { /* @@ -810,7 +767,7 @@ auto symcache_runtime::process_item_rdeps(struct rspamd_task *task, cache_item * for (const auto &rdep: item->rdeps) { if (rdep.item) { - auto *dyn_item = get_dynamic_item(rdep.item->id, true); + auto *dyn_item = get_dynamic_item(rdep.item->id); if (!dyn_item->started) { msg_debug_cache_task ("check item %d(%s) rdep of %s ", rdep.item->id, rdep.item->symbol.c_str(), item->symbol.c_str()); diff --git a/src/libserver/symcache/symcache_runtime.hxx b/src/libserver/symcache/symcache_runtime.hxx index 678dc42a6..3581a5383 100644 --- a/src/libserver/symcache/symcache_runtime.hxx +++ b/src/libserver/symcache/symcache_runtime.hxx @@ -58,8 +58,6 @@ class symcache_runtime { struct cache_dynamic_item *cur_item; order_generation_ptr order; - /* Cache of the last items to speed up lookups */ - mutable std::pair last_id_mappings[8]; /* Dynamically expanded as needed */ mutable struct cache_dynamic_item dynamic_items[]; /* We allocate this structure merely in memory pool, so destructor is absent */ @@ -166,7 +164,7 @@ public: * @param id * @return */ - auto get_dynamic_item(int id, bool save_in_cache) const -> cache_dynamic_item *; + auto get_dynamic_item(int id) const -> cache_dynamic_item *; /** * Returns static cache item by dynamic cache item @@ -188,7 +186,7 @@ public: * @param task * @param item */ - auto finalize_item(struct rspamd_task *task, cache_item *item) -> void; + auto finalize_item(struct rspamd_task *task, cache_dynamic_item *item) -> void; /** * Process unblocked reverse dependencies of the specific item -- 2.39.5