From 977d6f8f42f1d33a698417593c62d94938abc9c9 Mon Sep 17 00:00:00 2001
From: "dgrowley@gmail.com" <dgrowley@gmail.com>
Date: Thu, 2 Jul 2020 19:07:34 +1200
Subject: [PATCH v6 2/3] Allow users of simplehash.h to perform direct
 deletions

Previously simplehash.h only exposed a method to perform a hash table
delete by the key.  This required performing a hash table lookup in order
to find the element which belongs to that key.  Having the code this way
made sense for the existing callers, but in an up-coming commit, a caller
already has the element which it would like to delete, so can do so
without performing a lookup.
---
 src/include/lib/simplehash.h | 116 +++++++++++++++++++----------------
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h
index 395be1ca9a..e7df323de5 100644
--- a/src/include/lib/simplehash.h
+++ b/src/include/lib/simplehash.h
@@ -110,6 +110,7 @@
 #define SH_RESET SH_MAKE_NAME(reset)
 #define SH_INSERT SH_MAKE_NAME(insert)
 #define SH_INSERT_HASH SH_MAKE_NAME(insert_hash)
+#define SH_DELETE_ITEM SH_MAKE_NAME(delete_item)
 #define SH_DELETE SH_MAKE_NAME(delete)
 #define SH_LOOKUP SH_MAKE_NAME(lookup)
 #define SH_LOOKUP_HASH SH_MAKE_NAME(lookup_hash)
@@ -217,6 +218,9 @@ SH_SCOPE	SH_ELEMENT_TYPE *SH_LOOKUP(SH_TYPE * tb, SH_KEY_TYPE key);
 SH_SCOPE	SH_ELEMENT_TYPE *SH_LOOKUP_HASH(SH_TYPE * tb, SH_KEY_TYPE key,
 											uint32 hash);
 
+/* void <prefix>_delete_item(<prefix>_hash *tb, <element> *entry) */
+SH_SCOPE void SH_DELETE_ITEM(SH_TYPE * tb, SH_ELEMENT_TYPE * entry);
+
 /* bool <prefix>_delete(<prefix>_hash *tb, <key> key) */
 SH_SCOPE bool SH_DELETE(SH_TYPE * tb, SH_KEY_TYPE key);
 
@@ -829,75 +833,80 @@ SH_LOOKUP_HASH(SH_TYPE * tb, SH_KEY_TYPE key, uint32 hash)
 }
 
 /*
- * Delete entry from hash table.  Returns whether to-be-deleted key was
- * present.
+ * Delete 'entry' from hash table.
  */
-SH_SCOPE bool
-SH_DELETE(SH_TYPE * tb, SH_KEY_TYPE key)
+SH_SCOPE void
+SH_DELETE_ITEM(SH_TYPE * tb, SH_ELEMENT_TYPE * entry)
 {
-	uint32		hash = SH_HASH_KEY(tb, key);
+	SH_ELEMENT_TYPE *lastentry = entry;
+	uint32		hash = SH_ENTRY_HASH(tb, entry);
 	uint32		startelem = SH_INITIAL_BUCKET(tb, hash);
-	uint32		curelem = startelem;
+	uint32		curelem;
 
+	/* Calculate the index of 'entry' */
+	curelem = entry - &tb->data[0];
+
+	tb->members--;
+
+	/*
+	 * Backward shift following elements till either an empty element
+	 * or an element at its optimal position is encountered.
+	 *
+	 * While that sounds expensive, the average chain length is short,
+	 * and deletions would otherwise require tombstones.
+	 */
 	while (true)
 	{
-		SH_ELEMENT_TYPE *entry = &tb->data[curelem];
+		SH_ELEMENT_TYPE *curentry;
+		uint32		curhash;
+		uint32		curoptimal;
 
-		if (entry->status == SH_STATUS_EMPTY)
-			return false;
+		curelem = SH_NEXT(tb, curelem, startelem);
+		curentry = &tb->data[curelem];
 
-		if (entry->status == SH_STATUS_IN_USE &&
-			SH_COMPARE_KEYS(tb, hash, key, entry))
+		if (curentry->status != SH_STATUS_IN_USE)
 		{
-			SH_ELEMENT_TYPE *lastentry = entry;
-
-			tb->members--;
-
-			/*
-			 * Backward shift following elements till either an empty element
-			 * or an element at its optimal position is encountered.
-			 *
-			 * While that sounds expensive, the average chain length is short,
-			 * and deletions would otherwise require tombstones.
-			 */
-			while (true)
-			{
-				SH_ELEMENT_TYPE *curentry;
-				uint32		curhash;
-				uint32		curoptimal;
-
-				curelem = SH_NEXT(tb, curelem, startelem);
-				curentry = &tb->data[curelem];
-
-				if (curentry->status != SH_STATUS_IN_USE)
-				{
-					lastentry->status = SH_STATUS_EMPTY;
-					break;
-				}
-
-				curhash = SH_ENTRY_HASH(tb, curentry);
-				curoptimal = SH_INITIAL_BUCKET(tb, curhash);
+			lastentry->status = SH_STATUS_EMPTY;
+			break;
+		}
 
-				/* current is at optimal position, done */
-				if (curoptimal == curelem)
-				{
-					lastentry->status = SH_STATUS_EMPTY;
-					break;
-				}
+		curhash = SH_ENTRY_HASH(tb, curentry);
+		curoptimal = SH_INITIAL_BUCKET(tb, curhash);
 
-				/* shift */
-				memcpy(lastentry, curentry, sizeof(SH_ELEMENT_TYPE));
+		/* current is at optimal position, done */
+		if (curoptimal == curelem)
+		{
+			lastentry->status = SH_STATUS_EMPTY;
+			break;
+		}
 
-				lastentry = curentry;
-			}
+		/* shift */
+		memcpy(lastentry, curentry, sizeof(SH_ELEMENT_TYPE));
 
-			return true;
-		}
+		lastentry = curentry;
+	}
+}
 
-		/* TODO: return false; if distance too big */
+/*
+ * Perform hash table lookup on 'key', delete the entry belonging to it and
+ * return true.  Returns false if no item could be found relating to 'key'.
+ */
+SH_SCOPE bool
+SH_DELETE(SH_TYPE * tb, SH_KEY_TYPE key)
+{
+	SH_ELEMENT_TYPE *entry = SH_LOOKUP(tb, key);
 
-		curelem = SH_NEXT(tb, curelem, startelem);
+	if (likely(entry != NULL))
+	{
+		/*
+		 * Perform deletion and also the relocation of subsequent items which
+		 * are not in their optimal position but can now be moved up.
+		 */
+		SH_DELETE_ITEM(tb, entry);
+		return true;
 	}
+
+	return false;		/* Can't find 'key' */
 }
 
 /*
@@ -1102,6 +1111,7 @@ SH_STAT(SH_TYPE * tb)
 #undef SH_RESET
 #undef SH_INSERT
 #undef SH_INSERT_HASH
+#undef SH_DELETE_ITEM
 #undef SH_DELETE
 #undef SH_LOOKUP
 #undef SH_LOOKUP_HASH
-- 
2.21.0.windows.1

