From f2a2d01ac3ef0893464dd62e3742c0960b78ac62 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 2 Aug 2012 18:50:32 +0000
Subject: [PATCH] De-obfuscate deletion code, maybe.

---
 src/backend/utils/hash/chash.c | 83 ++++++++++++++++------------------
 1 file changed, 39 insertions(+), 44 deletions(-)

diff --git a/src/backend/utils/hash/chash.c b/src/backend/utils/hash/chash.c
index d82582d952..b3281d50f5 100644
--- a/src/backend/utils/hash/chash.c
+++ b/src/backend/utils/hash/chash.c
@@ -180,6 +180,7 @@ typedef struct CHashTableData
 typedef struct
 {
 	CHashPtr	target;
+	CHashPtr	next;
 	CHashPtr   *pointer_to_target;
 	CHashNode  *target_node;
 	bool		found;
@@ -547,52 +548,43 @@ CHashDelete(CHashTable table, void *entry)
 	pg_memory_barrier();
 
 	/* Scan bucket. */
+retry:
 	CHashBucketScan(table, b, hashcode, entry, &scan);
 
-	/*
-	 * If we found a match, then loop until we succesfully delete-mark the
-	 * target, or until someone else does so.  There's no real risk of
-	 * starvation here; the maximum number of times we can iterate here is
-	 * equal to the number of backends other than this one, and we'd have
-	 * to be very unlucky to have things turn out even that badly.
-	 */
-	while (scan.found)
+	/* If we found it, try to delete it. */
+	if (scan.found)
 	{
-		CHashPtr	next = scan.target_node->next;
+		Assert(!CHashPtrIsMarked(scan.next));
 
-		if (CHashPtrIsMarked(scan.target))
+		/* Attempt to apply delete-mark. */
+		if (!__sync_bool_compare_and_swap(&scan.target_node->next,
+										  scan.next,
+										  CHashPtrMark(scan.next)))
 		{
-			/* Someone else deleted it before we did. */
-			scan.found = false;
+			CHashTableIncrementStatistic(table, CHS_Delete_Retry);
+			goto retry;
 		}
-		else if (__sync_bool_compare_and_swap(&scan.target_node->next,
-											  next,
-											  CHashPtrMark(next)))
+
+		/* Deletion is done; attempt to remove node from list. */
+		if (__sync_bool_compare_and_swap(scan.pointer_to_target,
+										 scan.target,
+										 scan.next))
+			CHashAddToGarbage(table, bucket, scan.target);
+		else
 		{
-			/* Deletion is done; attempt to remove node from list. */
-			Assert(!CHashPtrIsMarked(scan.target));
-			if (__sync_bool_compare_and_swap(scan.pointer_to_target,
-											 scan.target,
-											 CHashPtrUnmark(next)))
-				CHashAddToGarbage(table, bucket, scan.target);
-			else
-			{
-				CHashScanResult	cleanup_scan;
+			CHashScanResult	cleanup_scan;
 
-				/*
-				 * If we weren't able to remove the deleted item, rescan
-				 * the bucket to make sure it's really gone.  This is just
-				 * like a regular bucket scan, except that we don't care
-				 * about the results.  We're just doing it to achieve the
-				 * side-effect of removing delete-marked nodes from the
-				 * bucket chain.
-				 */
-				CHashTableIncrementStatistic(table, CHS_Cleanup_Scan);
-				CHashBucketScan(table, b, hashcode, entry, &cleanup_scan);
-			}
-			break;
+			/*
+			 * If we weren't able to remove the deleted item, rescan
+			 * the bucket to make sure it's really gone.  This is just
+			 * like a regular bucket scan, except that we don't care
+			 * about the results.  We're just doing it to achieve the
+			 * side-effect of removing delete-marked nodes from the
+			 * bucket chain.
+			 */
+			CHashTableIncrementStatistic(table, CHS_Cleanup_Scan);
+			CHashBucketScan(table, b, hashcode, entry, &cleanup_scan);
 		}
-		CHashTableIncrementStatistic(table, CHS_Delete_Retry);
 	}
 
 	/* Allow garbage collection for this bucket. */
@@ -623,13 +615,15 @@ CHashStatistics(CHashTable table, uint64 *stats)
  * Caller must suppress garbage collection for the target bucket before
  * calling this function, and resume it afterwards.
  *
- * If a matching item is found, res->target will be a CHashPtr to that item
- * and res->found will be true.  If not, res->target will be a CHashPtr to the
- * item which follows the insert position in the bucket chain, and res->found
- * will be false.  In either case, res->pointer_to_target will a pointer to
- * the address where the value of target was found.  res->target_node will be
- * a pointer to the address of the CHashNode referenced by res->target, unless
- * res->target is invalid.
+ * On return, res->found will be true if a matching item was found and false
+ * otherwise.  res->target will be a CHashPtr referencing the matching item,
+ * or the first one following the position where the matching item should have
+ * been; res->pointer_to_target will be a pointer to the memory address from
+ * which res->target was read.
+ *
+ * If res->target is not invalid, then res->target_node is a pointer to its
+ * location in memory.  If res->found, then res->next will be the next pointer
+ * of res->target_node; otherwise, it's undefined.
  */
 static void
 CHashBucketScan(CHashTable table,
@@ -759,6 +753,7 @@ zap:
 			if (cmp == 0)
 			{
 				res->found = true;
+				res->next = next;
 				break;
 			}
 			else
-- 
2.39.5