commit f352f18: [Rework] Use dynamic items in the callbacks

Vsevolod Stakhov vsevolod at rspamd.com
Mon May 9 20:35:04 UTC 2022


Author: Vsevolod Stakhov
Date: 2022-05-09 21:32:35 +0100
URL: https://github.com/rspamd/rspamd/commit/f352f18cfb1448386a9c32a06042f4d94be13b27 (HEAD -> master)

[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<int, int> 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


More information about the Commits mailing list