De-obfuscate deletion code, maybe.

diff --git a/src/backend/utils/hash/chash.c b/src/backend/utils/hash/chash.c
index d82582d952f8f96e449a53df352bd544e58c2789..b3281d50f5620f7d44ca72d62508199ed34a7051 100644 (file)
--- 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