Merge pull request #826 from sakateka/lruhash_max_collisions

wcawijngaards · web-flow · commit 785c938106db · 2023-01-13T10:57:44.000+01:00
Аdd a metric about the maximum number of collisions in lrushah
diff --git a/daemon/remote.c b/daemon/remote.c
@@ -1065,6 +1065,11 @@ print_ext(RES* ssl, struct ub_stats_info* s, int inhibit_zero)
 		(unsigned)s->svr.infra_cache_count)) return 0;
 	if(!ssl_printf(ssl, "key.cache.count"SQ"%u\n",
 		(unsigned)s->svr.key_cache_count)) return 0;
+	/* max collisions */
+	if(!ssl_printf(ssl, "msg.cache.max_collisions"SQ"%u\n",
+		(unsigned)s->svr.msg_cache_max_collisions)) return 0;
+	if(!ssl_printf(ssl, "rrset.cache.max_collisions"SQ"%u\n",
+		(unsigned)s->svr.rrset_cache_max_collisions)) return 0;
 	/* applied RPZ actions */
 	for(i=0; i<UB_STATS_RPZ_ACTION_NUM; i++) {
 		if(i == RPZ_NO_OVERRIDE_ACTION)
diff --git a/daemon/stats.c b/daemon/stats.c
@@ -293,8 +293,10 @@ server_stats_compile(struct worker* worker, struct ub_stats_info* s, int reset)
 	s->svr.queries_ratelimited = (long long)get_queries_ratelimit(worker, reset);
 
 	/* get cache sizes */
-	s->svr.msg_cache_count = (long long)count_slabhash_entries(worker->env.msg_cache);
-	s->svr.rrset_cache_count = (long long)count_slabhash_entries(&worker->env.rrset_cache->table);
+	get_slabhash_stats(worker->env.msg_cache,
+		&s->svr.msg_cache_count, &s->svr.msg_cache_max_collisions);
+	get_slabhash_stats(&worker->env.rrset_cache->table,
+		&s->svr.rrset_cache_count, &s->svr.rrset_cache_max_collisions);
 	s->svr.infra_cache_count = (long long)count_slabhash_entries(worker->env.infra_cache->hosts);
 	if(worker->env.key_cache)
 		s->svr.key_cache_count = (long long)count_slabhash_entries(worker->env.key_cache->slab);
diff --git a/libunbound/unbound.h b/libunbound/unbound.h
@@ -788,6 +788,11 @@ struct ub_server_stats {
 	/** number of key cache entries */
 	long long key_cache_count;
 
+	/** maximum number of collisions in the msg cache */
+	long long msg_cache_max_collisions;
+	/** maximum number of collisions in the rrset cache */
+	long long rrset_cache_max_collisions;
+
 	/** number of queries that used dnscrypt */
 	long long num_query_dnscrypt_crypted;
 	/** number of queries that queried dnscrypt certificates */
diff --git a/smallapp/unbound-control.c b/smallapp/unbound-control.c
@@ -403,6 +403,9 @@ static void print_extended(struct ub_stats_info* s, int inhibit_zero)
 	PR_UL("rrset.cache.count", s->svr.rrset_cache_count);
 	PR_UL("infra.cache.count", s->svr.infra_cache_count);
 	PR_UL("key.cache.count", s->svr.key_cache_count);
+	/* max collisions */
+	PR_UL("msg.cache.max_collisions", s->svr.msg_cache_max_collisions);
+	PR_UL("rrset.cache.max_collisions", s->svr.rrset_cache_max_collisions);
 	/* applied RPZ actions */
 	for(i=0; i<UB_STATS_RPZ_ACTION_NUM; i++) {
 		if(i == RPZ_NO_OVERRIDE_ACTION)
diff --git a/testcode/unitlruhash.c b/testcode/unitlruhash.c
@@ -94,28 +94,28 @@ test_bin_find_entry(struct lruhash* table)
 	bin_overflow_remove(&bin, &k->entry);
 
 	/* find in empty list */
-	unit_assert( bin_find_entry(table, &bin, h, k) == NULL );
+	unit_assert( bin_find_entry(table, &bin, h, k, NULL) == NULL );
 
 	/* insert */
 	lock_quick_lock(&bin.lock);
 	bin.overflow_list = &k->entry;
 	lock_quick_unlock(&bin.lock);
 
 	/* find, hash not OK. */
-	unit_assert( bin_find_entry(table, &bin, myhash(13), k) == NULL );
+	unit_assert( bin_find_entry(table, &bin, myhash(13), k, NULL) == NULL );
 
 	/* find, hash OK, but cmp not */
 	unit_assert( k->entry.hash == k2->entry.hash );
-	unit_assert( bin_find_entry(table, &bin, h, k2) == NULL );
+	unit_assert( bin_find_entry(table, &bin, h, k2, NULL) == NULL );
 
 	/* find, hash OK, and cmp too */
-	unit_assert( bin_find_entry(table, &bin, h, k) == &k->entry );
+	unit_assert( bin_find_entry(table, &bin, h, k, NULL) == &k->entry );
 
 	/* remove the element */
 	lock_quick_lock(&bin.lock);
 	bin_overflow_remove(&bin, &k->entry);
 	lock_quick_unlock(&bin.lock);
-	unit_assert( bin_find_entry(table, &bin, h, k) == NULL );
+	unit_assert( bin_find_entry(table, &bin, h, k, NULL) == NULL );
 
 	/* prepend two different elements; so the list is long */
 	/* one has the same hash, but different cmp */
@@ -127,28 +127,28 @@ test_bin_find_entry(struct lruhash* table)
 	lock_quick_unlock(&bin.lock);
 
 	/* find, hash not OK. */
-	unit_assert( bin_find_entry(table, &bin, myhash(13), k) == NULL );
+	unit_assert( bin_find_entry(table, &bin, myhash(13), k, NULL) == NULL );
 
 	/* find, hash OK, but cmp not */
 	unit_assert( k->entry.hash == k2->entry.hash );
-	unit_assert( bin_find_entry(table, &bin, h, k2) == NULL );
+	unit_assert( bin_find_entry(table, &bin, h, k2, NULL) == NULL );
 
 	/* find, hash OK, and cmp too */
-	unit_assert( bin_find_entry(table, &bin, h, k) == &k->entry );
+	unit_assert( bin_find_entry(table, &bin, h, k, NULL) == &k->entry );
 
 	/* remove middle element */
-	unit_assert( bin_find_entry(table, &bin, k4->entry.hash, k4) 
+	unit_assert( bin_find_entry(table, &bin, k4->entry.hash, k4, NULL)
 		== &k4->entry );
 	lock_quick_lock(&bin.lock);
 	bin_overflow_remove(&bin, &k4->entry);
 	lock_quick_unlock(&bin.lock);
-	unit_assert( bin_find_entry(table, &bin, k4->entry.hash, k4) == NULL);
+	unit_assert( bin_find_entry(table, &bin, k4->entry.hash, k4, NULL) == NULL);
 
 	/* remove last element */
 	lock_quick_lock(&bin.lock);
 	bin_overflow_remove(&bin, &k->entry);
 	lock_quick_unlock(&bin.lock);
-	unit_assert( bin_find_entry(table, &bin, h, k) == NULL );
+	unit_assert( bin_find_entry(table, &bin, h, k, NULL) == NULL );
 
 	lock_quick_destroy(&bin.lock);
 	delkey(k);
diff --git a/util/storage/lruhash.c b/util/storage/lruhash.c
@@ -81,6 +81,7 @@ lruhash_create(size_t start_size, size_t maxmem,
 	table->num = 0;
 	table->space_used = 0;
 	table->space_max = maxmem;
+	table->max_collisions = 0;
 	table->array = calloc(table->size, sizeof(struct lruhash_bin));
 	if(!table->array) {
 		lock_quick_destroy(&table->lock);
@@ -216,15 +217,19 @@ reclaim_space(struct lruhash* table, struct lruhash_entry** list)
 
 struct lruhash_entry* 
 bin_find_entry(struct lruhash* table, 
-	struct lruhash_bin* bin, hashvalue_type hash, void* key)
+	struct lruhash_bin* bin, hashvalue_type hash, void* key, size_t* collisions)
 {
+	size_t c = 0;
 	struct lruhash_entry* p = bin->overflow_list;
 	while(p) {
 		if(p->hash == hash && table->compfunc(p->key, key) == 0)
-			return p;
+			break;
+		c++;
 		p = p->overflow_next;
 	}
-	return NULL;
+	if (collisions != NULL)
+		*collisions = c;
+	return p;
 }
 
 void 
@@ -303,6 +308,7 @@ lruhash_insert(struct lruhash* table, hashvalue_type hash,
 	struct lruhash_bin* bin;
 	struct lruhash_entry* found, *reclaimlist=NULL;
 	size_t need_size;
+	size_t collisions;
 	fptr_ok(fptr_whitelist_hash_sizefunc(table->sizefunc));
 	fptr_ok(fptr_whitelist_hash_delkeyfunc(table->delkeyfunc));
 	fptr_ok(fptr_whitelist_hash_deldatafunc(table->deldatafunc));
@@ -317,12 +323,14 @@ lruhash_insert(struct lruhash* table, hashvalue_type hash,
 	lock_quick_lock(&bin->lock);
 
 	/* see if entry exists already */
-	if(!(found=bin_find_entry(table, bin, hash, entry->key))) {
+	if(!(found=bin_find_entry(table, bin, hash, entry->key, &collisions))) {
 		/* if not: add to bin */
 		entry->overflow_next = bin->overflow_list;
 		bin->overflow_list = entry;
 		lru_front(table, entry);
 		table->num++;
+		if (table->max_collisions < collisions)
+			table->max_collisions = collisions;
 		table->space_used += need_size;
 	} else {
 		/* if so: update data - needs a writelock */
@@ -362,7 +370,7 @@ lruhash_lookup(struct lruhash* table, hashvalue_type hash, void* key, int wr)
 	lock_quick_lock(&table->lock);
 	bin = &table->array[hash & table->size_mask];
 	lock_quick_lock(&bin->lock);
-	if((entry=bin_find_entry(table, bin, hash, key)))
+	if((entry=bin_find_entry(table, bin, hash, key, NULL)))
 		lru_touch(table, entry);
 	lock_quick_unlock(&table->lock);
 
@@ -389,7 +397,7 @@ lruhash_remove(struct lruhash* table, hashvalue_type hash, void* key)
 	lock_quick_lock(&table->lock);
 	bin = &table->array[hash & table->size_mask];
 	lock_quick_lock(&bin->lock);
-	if((entry=bin_find_entry(table, bin, hash, key))) {
+	if((entry=bin_find_entry(table, bin, hash, key, NULL))) {
 		bin_overflow_remove(bin, entry);
 		lru_remove(table, entry);
 	} else {
@@ -579,6 +587,7 @@ lruhash_insert_or_retrieve(struct lruhash* table, hashvalue_type hash,
 	struct lruhash_bin* bin;
 	struct lruhash_entry* found, *reclaimlist = NULL;
 	size_t need_size;
+	size_t collisions;
 	fptr_ok(fptr_whitelist_hash_sizefunc(table->sizefunc));
 	fptr_ok(fptr_whitelist_hash_delkeyfunc(table->delkeyfunc));
 	fptr_ok(fptr_whitelist_hash_deldatafunc(table->deldatafunc));
@@ -593,7 +602,7 @@ lruhash_insert_or_retrieve(struct lruhash* table, hashvalue_type hash,
 	lock_quick_lock(&bin->lock);
 
 	/* see if entry exists already */
-	if ((found = bin_find_entry(table, bin, hash, entry->key)) != NULL) {
+	if ((found = bin_find_entry(table, bin, hash, entry->key, &collisions)) != NULL) {
 		/* if so: keep the existing data - acquire a writelock */
 		lock_rw_wrlock(&found->lock);
 	}
@@ -604,6 +613,8 @@ lruhash_insert_or_retrieve(struct lruhash* table, hashvalue_type hash,
 		bin->overflow_list = entry;
 		lru_front(table, entry);
 		table->num++;
+		if (table->max_collisions < collisions)
+			table->max_collisions = collisions;
 		table->space_used += need_size;
 		/* return the entry that was presented, and lock it */
 		found = entry;
diff --git a/util/storage/lruhash.h b/util/storage/lruhash.h
@@ -178,6 +178,8 @@ struct lruhash {
 	size_t space_used;
 	/** the amount of space the hash table is maximally allowed to use. */
 	size_t space_max;
+	/** the maximum collisions were detected during the lruhash_insert operations. */
+	size_t max_collisions;
 };
 
 /**
@@ -357,10 +359,11 @@ void bin_delete(struct lruhash* table, struct lruhash_bin* bin);
  * @param bin: hash bin to look into.
  * @param hash: hash value to look for.
  * @param key: key to look for.
+ * @param collisions: how many collisions were found during the search.
  * @return: the entry or NULL if not found.
  */
 struct lruhash_entry* bin_find_entry(struct lruhash* table, 
-	struct lruhash_bin* bin, hashvalue_type hash, void* key);
+	struct lruhash_bin* bin, hashvalue_type hash, void* key, size_t* collisions);
 
 /**
  * Remove entry from bin overflow chain.
diff --git a/util/storage/slabhash.c b/util/storage/slabhash.c
@@ -242,3 +242,21 @@ size_t count_slabhash_entries(struct slabhash* sh)
 	}
 	return cnt;
 }
+
+void get_slabhash_stats(struct slabhash* sh, long long* num, long long* collisions)
+{
+	size_t slab, cnt = 0, max_collisions = 0;
+
+	for(slab=0; slab<sh->size; slab++) {
+		lock_quick_lock(&sh->array[slab]->lock);
+		cnt += sh->array[slab]->num;
+		if (max_collisions < sh->array[slab]->max_collisions) {
+			max_collisions = sh->array[slab]->max_collisions;
+		}
+		lock_quick_unlock(&sh->array[slab]->lock);
+	}
+	if (num != NULL)
+		*num = cnt;
+	if (collisions != NULL)
+		*collisions = max_collisions;
+}
diff --git a/util/storage/slabhash.h b/util/storage/slabhash.h
@@ -200,6 +200,15 @@ void slabhash_traverse(struct slabhash* table, int wr,
  */
 size_t count_slabhash_entries(struct slabhash* table);
 
+/**
+ * Retrieves number of items in slabhash and the current max collision level
+ * @param table: slabbed hash table.
+ * @param entries_count: where to save the current number of elements.
+ * @param max_collisions: where to save the current max collisions level.
+ */
+void get_slabhash_stats(struct slabhash* table,
+	long long* entries_count, long long* max_collisions);
+
 /* --- test representation --- */
 /** test structure contains test key */
 struct slabhash_testkey {
