From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 12 May 2017 23:05:13 +0000 (-0400)
Subject: Avoid searching for callback functions in CallSyscacheCallbacks().
X-Git-Tag: REL_10_BETA1~33
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2df5d465558b6f17c161cbbe246b050b453ec99c;p=postgresql

Avoid searching for callback functions in CallSyscacheCallbacks().

We have now grown enough registerable syscache-invalidation callback
functions that the original assumption that there would be few of them
is causing performance problems.  In particular, let's fix things so that
CallSyscacheCallbacks doesn't have to search the whole array to find
which callback(s) to invoke for a given cache ID.  Preserve the original
behavior that callbacks are called in order of registration, just in
case there's someplace that depends on that (which I doubt).

In support of this, export the number of syscaches from syscache.h.
People could have found that out anyway from the enum, but adding a
#define makes that much safer.

This provides a useful additional speedup in Mathieu Fenniak's
logical-decoding test case, although we're reaching the point of
diminishing returns there.  I think any further improvement will have
to come from reducing the number of cache invalidations that are
triggered in the first place.  Still, we can hope that this change
gives some incremental benefit for all invalidation scenarios.

Back-patch to 9.4 where logical decoding was introduced.

Discussion: https://postgr.es/m/CAHoiPjzea6N0zuCi=+f9v_j94nfsy6y8SU7-=bp4=7qw6_i=Rg@mail.gmail.com
---

diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index 72c8e9e0c4..a1e6ea2a35 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -177,7 +177,12 @@ static int	maxSharedInvalidMessagesArray;
 
 /*
  * Dynamically-registered callback functions.  Current implementation
- * assumes there won't be very many of these at once; could improve if needed.
+ * assumes there won't be enough of these to justify a dynamically resizable
+ * array; it'd be easy to improve that if needed.
+ *
+ * To avoid searching in CallSyscacheCallbacks, all callbacks for a given
+ * syscache are linked into a list pointed to by syscache_callback_links[id].
+ * The link values are syscache_callback_list[] index plus 1, or 0 for none.
  */
 
 #define MAX_SYSCACHE_CALLBACKS 64
@@ -186,10 +191,13 @@ static int	maxSharedInvalidMessagesArray;
 static struct SYSCACHECALLBACK
 {
 	int16		id;				/* cache number */
+	int16		link;			/* next callback index+1 for same cache */
 	SyscacheCallbackFunction function;
 	Datum		arg;
 }	syscache_callback_list[MAX_SYSCACHE_CALLBACKS];
 
+static int16 syscache_callback_links[SysCacheSize];
+
 static int	syscache_callback_count = 0;
 
 static struct RELCACHECALLBACK
@@ -1383,10 +1391,28 @@ CacheRegisterSyscacheCallback(int cacheid,
 							  SyscacheCallbackFunction func,
 							  Datum arg)
 {
+	if (cacheid < 0 || cacheid >= SysCacheSize)
+		elog(FATAL, "invalid cache ID: %d", cacheid);
 	if (syscache_callback_count >= MAX_SYSCACHE_CALLBACKS)
 		elog(FATAL, "out of syscache_callback_list slots");
 
+	if (syscache_callback_links[cacheid] == 0)
+	{
+		/* first callback for this cache */
+		syscache_callback_links[cacheid] = syscache_callback_count + 1;
+	}
+	else
+	{
+		/* add to end of chain, so that older callbacks are called first */
+		int			i = syscache_callback_links[cacheid] - 1;
+
+		while (syscache_callback_list[i].link > 0)
+			i = syscache_callback_list[i].link - 1;
+		syscache_callback_list[i].link = syscache_callback_count + 1;
+	}
+
 	syscache_callback_list[syscache_callback_count].id = cacheid;
+	syscache_callback_list[syscache_callback_count].link = 0;
 	syscache_callback_list[syscache_callback_count].function = func;
 	syscache_callback_list[syscache_callback_count].arg = arg;
 
@@ -1426,11 +1452,16 @@ CallSyscacheCallbacks(int cacheid, uint32 hashvalue)
 {
 	int			i;
 
-	for (i = 0; i < syscache_callback_count; i++)
+	if (cacheid < 0 || cacheid >= SysCacheSize)
+		elog(ERROR, "invalid cache ID: %d", cacheid);
+
+	i = syscache_callback_links[cacheid] - 1;
+	while (i >= 0)
 	{
 		struct SYSCACHECALLBACK *ccitem = syscache_callback_list + i;
 
-		if (ccitem->id == cacheid)
-			(*ccitem->function) (ccitem->arg, cacheid, hashvalue);
+		Assert(ccitem->id == cacheid);
+		(*ccitem->function) (ccitem->arg, cacheid, hashvalue);
+		i = ccitem->link - 1;
 	}
 }
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 066ce72877..f0a16e309c 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -971,8 +971,6 @@ static const struct cachedesc cacheinfo[] = {
 	}
 };
 
-#define SysCacheSize	((int) lengthof(cacheinfo))
-
 static CatCache *SysCache[SysCacheSize];
 
 static bool CacheInitialized = false;
@@ -1003,6 +1001,9 @@ InitCatalogCache(void)
 	int			i,
 				j;
 
+	StaticAssertStmt(SysCacheSize == (int) lengthof(cacheinfo),
+					 "SysCacheSize does not match syscache.c's array");
+
 	Assert(!CacheInitialized);
 
 	SysCacheRelationOidSize = SysCacheSupportingRelOidSize = 0;
diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h
index 73991dde4f..e20284d061 100644
--- a/src/include/utils/syscache.h
+++ b/src/include/utils/syscache.h
@@ -108,6 +108,8 @@ enum SysCacheIdentifier
 	TYPEOID,
 	USERMAPPINGOID,
 	USERMAPPINGUSERSERVER
+
+#define SysCacheSize (USERMAPPINGUSERSERVER + 1)
 };
 
 extern void InitCatalogCache(void);