Subject: [PATCH v6 2/3] Remove "dead" flag from catcache tuple

From: Kyotaro Horiguchi <[email-masked]>
Date: Wed, 18 Nov 2020 16:57:05 +0900

---
 src/backend/utils/cache/catcache.c | 43 +++++++++++++-----------------
 src/include/utils/catcache.h       | 10 -------
 2 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 644d92dd9a..611b65168d 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -480,6 +480,13 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
 	Assert(ct->refcount == 0);
 	Assert(ct->my_cache == cache);
 
+	/* delink from linked list if not yet */
+	if (ct->cache_elem.prev)
+	{
+		dlist_delete(&ct->cache_elem);
+		ct->cache_elem.prev = NULL;
+	}
+
 	if (ct->c_list)
 	{
 		/*
@@ -487,14 +494,10 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct)
 		 * which will recurse back to me, and the recursive call will do the
 		 * work.  Set the "dead" flag to make sure it does recurse.
 		 */
-		ct->dead = true;
 		CatCacheRemoveCList(cache, ct->c_list);
 		return;					/* nothing left to do */
 	}
 
-	/* delink from linked list */
-	dlist_delete(&ct->cache_elem);
-
 	/*
 	 * Free keys when we're dealing with a negative entry, normal entries just
 	 * point into tuple, allocated together with the CatCTup.
@@ -534,7 +537,7 @@ CatCacheRemoveCList(CatCache *cache, CatCList *cl)
 		/* if the member is dead and now has no references, remove it */
 		if (
 #ifndef CATCACHE_FORCE_RELEASE
-			ct->dead &&
+			ct->cache_elem.prev == NULL &&
 #endif
 			ct->refcount == 0)
 			CatCacheRemoveCTup(cache, ct);
@@ -609,7 +612,9 @@ CatCacheInvalidate(CatCache *cache, uint32 hashValue)
 			if (ct->refcount > 0 ||
 				(ct->c_list && ct->c_list->refcount > 0))
 			{
-				ct->dead = true;
+				dlist_delete(&ct->cache_elem);
+				ct->cache_elem.prev = NULL;
+
 				/* list, if any, was marked dead above */
 				Assert(ct->c_list == NULL || ct->c_list->dead);
 			}
@@ -688,7 +693,8 @@ ResetCatalogCache(CatCache *cache)
 			if (ct->refcount > 0 ||
 				(ct->c_list && ct->c_list->refcount > 0))
 			{
-				ct->dead = true;
+				dlist_delete(&ct->cache_elem);
+				ct->cache_elem.prev = NULL;
 				/* list, if any, was marked dead above */
 				Assert(ct->c_list == NULL || ct->c_list->dead);
 			}
@@ -1268,9 +1274,6 @@ SearchCatCacheInternal(CatCache *cache,
 	{
 		ct = dlist_container(CatCTup, cache_elem, iter.cur);
 
-		if (ct->dead)
-			continue;			/* ignore dead entries */
-
 		if (ct->hash_value != hashValue)
 			continue;			/* quickly skip entry if wrong hash val */
 
@@ -1522,7 +1525,6 @@ ReleaseCatCache(HeapTuple tuple)
 								  offsetof(CatCTup, tuple));
 
 	/* Safety checks to ensure we were handed a cache entry */
-	Assert(ct->ct_magic == CT_MAGIC);
 	Assert(ct->refcount > 0);
 
 	ct->refcount--;
@@ -1530,7 +1532,7 @@ ReleaseCatCache(HeapTuple tuple)
 
 	if (
 #ifndef CATCACHE_FORCE_RELEASE
-		ct->dead &&
+		ct->cache_elem.prev == NULL &&
 #endif
 		ct->refcount == 0 &&
 		(ct->c_list == NULL || ct->c_list->refcount == 0))
@@ -1737,8 +1739,8 @@ SearchCatCacheList(CatCache *cache,
 			{
 				ct = dlist_container(CatCTup, cache_elem, iter.cur);
 
-				if (ct->dead || ct->negative)
-					continue;	/* ignore dead and negative entries */
+				if (ct->negative)
+					continue;	/* ignore negative entries */
 
 				if (ct->hash_value != hashValue)
 					continue;	/* quickly skip entry if wrong hash val */
@@ -1799,14 +1801,13 @@ SearchCatCacheList(CatCache *cache,
 	{
 		foreach(ctlist_item, ctlist)
 		{
+			Assert (ct->cache_elem.prev != NULL);
+
 			ct = (CatCTup *) lfirst(ctlist_item);
 			Assert(ct->c_list == NULL);
 			Assert(ct->refcount > 0);
 			ct->refcount--;
 			if (
-#ifndef CATCACHE_FORCE_RELEASE
-				ct->dead &&
-#endif
 				ct->refcount == 0 &&
 				(ct->c_list == NULL || ct->c_list->refcount == 0))
 				CatCacheRemoveCTup(cache, ct);
@@ -1834,9 +1835,6 @@ SearchCatCacheList(CatCache *cache,
 		/* release the temporary refcount on the member */
 		Assert(ct->refcount > 0);
 		ct->refcount--;
-		/* mark list dead if any members already dead */
-		if (ct->dead)
-			cl->dead = true;
 	}
 	Assert(i == nmembers);
 
@@ -1960,11 +1958,9 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
 	 * Finish initializing the CatCTup header, and add it to the cache's
 	 * linked list and counts.
 	 */
-	ct->ct_magic = CT_MAGIC;
 	ct->my_cache = cache;
 	ct->c_list = NULL;
 	ct->refcount = 0;			/* for the moment */
-	ct->dead = false;
 	ct->negative = negative;
 	ct->hash_value = hashValue;
 	ct->lastaccess = catcacheclock;
@@ -2158,9 +2154,6 @@ PrintCatCacheLeakWarning(HeapTuple tuple)
 	CatCTup    *ct = (CatCTup *) (((char *) tuple) -
 								  offsetof(CatCTup, tuple));
 
-	/* Safety check to ensure we were handed a cache entry */
-	Assert(ct->ct_magic == CT_MAGIC);
-
 	elog(WARNING, "cache reference leak: cache %s (%d), tuple %u/%u has count %d",
 		 ct->my_cache->cc_relname, ct->my_cache->id,
 		 ItemPointerGetBlockNumber(&(tuple->t_self)),
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index 291e857e38..53b0bf31eb 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -87,9 +87,6 @@ typedef struct catcache
 
 typedef struct catctup
 {
-	int			ct_magic;		/* for identifying CatCTup entries */
-#define CT_MAGIC   0x57261502
-
 	uint32		hash_value;		/* hash value for this tuple's keys */
 
 	/*
@@ -106,19 +103,12 @@ typedef struct catctup
 	dlist_node	cache_elem;		/* list member of per-bucket list */
 
 	/*
-	 * A tuple marked "dead" must not be returned by subsequent searches.
-	 * However, it won't be physically deleted from the cache until its
-	 * refcount goes to zero.  (If it's a member of a CatCList, the list's
-	 * refcount must go to zero, too; also, remember to mark the list dead at
-	 * the same time the tuple is marked.)
-	 *
 	 * A negative cache entry is an assertion that there is no tuple matching
 	 * a particular key.  This is just as useful as a normal entry so far as
 	 * avoiding catalog searches is concerned.  Management of positive and
 	 * negative entries is identical.
 	 */
 	int			refcount;		/* number of active references */
-	bool		dead;			/* dead but not yet removed? */
 	bool		negative;		/* negative cache entry? */
 	HeapTupleData tuple;		/* tuple management header */
 	uint64		lastaccess;		/* timestamp in us of the last usage */
-- 
2.27.0

----Next_Part(Thu_Jan_14_17_32_27_2021_995)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="v6-0003-catcachebench.patch"