Rearrange fmgr.c and relcache so that it's possible to keep FmgrInfo

lookup info in the relcache for index access method support functions.
This makes a huge difference for dynamically loaded support functions,
and should save a few cycles even for built-in ones.  Also tweak dfmgr.c
so that load_external_function is called only once, not twice, when
doing fmgr_info for a dynamically loaded function.  All per performance
gripe from Teodor Sigaev, 5-Oct-01.

5 files changed, 94 insertions, 111 deletions

diff --git a/src/backend/utils/cache/Makefile b/src/backend/utils/cache/Makefile
index 01896efdd3..5ac5a06827 100644
--- a/src/backend/utils/cache/Makefile
+++ b/src/backend/utils/cache/Makefile
@@ -4,7 +4,7 @@
 #    Makefile for utils/cache
 #
 # IDENTIFICATION
-#    $Header: /cvsroot/pgsql/src/backend/utils/cache/Makefile,v 1.14 2000/08/31 16:10:46 petere Exp $
+#    $Header: /cvsroot/pgsql/src/backend/utils/cache/Makefile,v 1.15 2001/10/06 23:21:44 tgl Exp $
 #
 #-------------------------------------------------------------------------
 
@@ -12,7 +12,7 @@ subdir = src/backend/utils/cache
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = catcache.o inval.o rel.o relcache.o syscache.o lsyscache.o \
+OBJS = catcache.o inval.o relcache.o syscache.o lsyscache.o \
 	fcache.o temprel.o
 
 all: SUBSYS.o
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index ce4363706b..77112a694e 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.82 2001/08/21 16:36:04 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/catcache.c,v 1.83 2001/10/06 23:21:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -269,23 +269,10 @@ CatalogCacheInitializeCache(CatCache *cache)
 		 */
 		cache->cc_skey[i].sk_procedure = EQPROC(keytype);
 
-		/*
-		 * Note: to avoid any possible leakage of scan temporary data into
-		 * the cache context, we do not switch into CacheMemoryContext while
-		 * calling fmgr_info here.  Instead set fn_mcxt on return.  This
-		 * would fail to work correctly if fmgr_info allocated any subsidiary
-		 * data structures to attach to the FmgrInfo record; but it doesn't
-		 * do so for built-in functions, and all the comparator functions
-		 * for system caches should most assuredly be built-in functions.
-		 * Currently there's no real need to fix fn_mcxt either, but let's do
-		 * that anyway just to make sure it's not pointing to a dead context
-		 * later on.
-		 */
-
-		fmgr_info(cache->cc_skey[i].sk_procedure,
-				  &cache->cc_skey[i].sk_func);
-
-		cache->cc_skey[i].sk_func.fn_mcxt = CacheMemoryContext;
+		/* Do function lookup */
+		fmgr_info_cxt(cache->cc_skey[i].sk_procedure,
+					  &cache->cc_skey[i].sk_func,
+					  CacheMemoryContext);
 
 		/* Initialize sk_attno suitably for HeapKeyTest() and heap scans */
 		cache->cc_skey[i].sk_attno = cache->cc_key[i];
diff --git a/src/backend/utils/cache/fcache.c b/src/backend/utils/cache/fcache.c
index bb1ac36f3e..92cf46a036 100644
--- a/src/backend/utils/cache/fcache.c
+++ b/src/backend/utils/cache/fcache.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/fcache.c,v 1.40 2001/09/21 00:11:31 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/fcache.c,v 1.41 2001/10/06 23:21:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,26 +23,22 @@
 FunctionCachePtr
 init_fcache(Oid foid, int nargs, MemoryContext fcacheCxt)
 {
-	MemoryContext oldcontext;
 	FunctionCachePtr retval;
 
 	/* Safety check (should never fail, as parser should check sooner) */
 	if (nargs > FUNC_MAX_ARGS)
 		elog(ERROR, "init_fcache: too many arguments");
 
-	/* Switch to a context long-lived enough for the fcache entry */
-	oldcontext = MemoryContextSwitchTo(fcacheCxt);
-
-	retval = (FunctionCachePtr) palloc(sizeof(FunctionCache));
+	/* Create fcache entry in the desired context */
+	retval = (FunctionCachePtr) MemoryContextAlloc(fcacheCxt,
+												   sizeof(FunctionCache));
 	MemSet(retval, 0, sizeof(FunctionCache));
 
 	/* Set up the primary fmgr lookup information */
-	fmgr_info(foid, &(retval->func));
+	fmgr_info_cxt(foid, &(retval->func), fcacheCxt);
 
 	/* Initialize additional info */
 	retval->setArgsValid = false;
 
-	MemoryContextSwitchTo(oldcontext);
-
 	return retval;
 }
diff --git a/src/backend/utils/cache/rel.c b/src/backend/utils/cache/rel.c
deleted file mode 100644
index 588566afc4..0000000000
--- a/src/backend/utils/cache/rel.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * rel.c
- *	  POSTGRES relation descriptor code.
- *
- * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *
- * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/Attic/rel.c,v 1.9 2001/01/24 19:43:15 momjian Exp $
- *
- *-------------------------------------------------------------------------
- */
-
-#include "postgres.h"
-#include "access/istrat.h"
-
-
-/*
- *		RelationIsValid is now a macro in rel.h -cim 4/27/91
- *
- *		All of the RelationGet...() functions are now macros in rel.h
- *				-mer 3/2/92
- */
-
-/*
- * RelationSetIndexSupport
- *		Sets index strategy and support info for a relation.
- *
- *		This routine saves two pointers -- one to the IndexStrategy, and
- *		one to the RegProcs that support the indexed access method.
- *
- * Note:
- *		Assumes relation descriptor is valid.
- *		Assumes index strategy is valid.  Assumes support is valid if non-
- *		NULL.
- */
-void
-RelationSetIndexSupport(Relation relation,
-						IndexStrategy strategy,
-						RegProcedure *support)
-{
-	Assert(PointerIsValid(relation));
-	Assert(IndexStrategyIsValid(strategy));
-
-	relation->rd_istrat = strategy;
-	relation->rd_support = support;
-}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 8d85487139..c56d606076 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.145 2001/10/05 17:28:12 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.146 2001/10/06 23:21:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -249,7 +249,6 @@ static void build_tupdesc_ind(RelationBuildDescInfo buildinfo,
 				  Relation relation);
 static Relation RelationBuildDesc(RelationBuildDescInfo buildinfo,
 				  Relation oldrelation);
-static void IndexedAccessMethodInitialize(Relation relation);
 static void AttrDefaultFetch(Relation relation);
 static void RelCheckFetch(Relation relation);
 static List *insert_ordered_oid(List *list, Oid datum);
@@ -1044,7 +1043,7 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
 	 * initialize index strategy and support information for this relation
 	 */
 	if (OidIsValid(relam))
-		IndexedAccessMethodInitialize(relation);
+		RelationInitIndexAccessInfo(relation);
 
 	/*
 	 * initialize the relation lock manager information
@@ -1087,41 +1086,75 @@ RelationBuildDesc(RelationBuildDescInfo buildinfo,
 	return relation;
 }
 
-static void
-IndexedAccessMethodInitialize(Relation relation)
+/*
+ * Initialize index-access-method support data for an index relation
+ */
+void
+RelationInitIndexAccessInfo(Relation relation)
 {
+	MemoryContext indexcxt;
 	IndexStrategy strategy;
 	RegProcedure *support;
+	FmgrInfo   *supportinfo;
 	int			natts;
-	Size		stratSize;
-	Size		supportSize;
 	uint16		amstrategies;
 	uint16		amsupport;
+	Size		stratSize;
 
 	natts = relation->rd_rel->relnatts;
 	amstrategies = relation->rd_am->amstrategies;
 	amsupport = relation->rd_am->amsupport;
 
+	/*
+	 * Make the private context to hold index access info.  The reason
+	 * we need a context, and not just a couple of pallocs, is so that
+	 * we won't leak any subsidiary info attached to fmgr lookup records.
+	 *
+	 * Context parameters are set on the assumption that it'll probably not
+	 * contain much data.
+	 */
+	indexcxt = AllocSetContextCreate(CacheMemoryContext,
+									 RelationGetRelationName(relation),
+									 0,			/* minsize */
+									 512,		/* initsize */
+									 1024);		/* maxsize */
+	relation->rd_indexcxt = indexcxt;
+
+	/*
+	 * Allocate arrays to hold data
+	 */
 	stratSize = AttributeNumberGetIndexStrategySize(natts, amstrategies);
-	strategy = (IndexStrategy) MemoryContextAlloc(CacheMemoryContext,
-												  stratSize);
+	strategy = (IndexStrategy) MemoryContextAlloc(indexcxt, stratSize);
 
 	if (amsupport > 0)
 	{
-		supportSize = natts * (amsupport * sizeof(RegProcedure));
-		support = (RegProcedure *) MemoryContextAlloc(CacheMemoryContext,
-													  supportSize);
+		int		nsupport = natts * amsupport;
+
+		support = (RegProcedure *)
+			MemoryContextAlloc(indexcxt, nsupport * sizeof(RegProcedure));
+		supportinfo = (FmgrInfo *)
+			MemoryContextAlloc(indexcxt, nsupport * sizeof(FmgrInfo));
+		MemSet(supportinfo, 0, nsupport * sizeof(FmgrInfo));
 	}
 	else
-		support = (RegProcedure *) NULL;
+	{
+		support = NULL;
+		supportinfo = NULL;
+	}
 
+	/*
+	 * Fill the strategy map and the support RegProcedure arrays.
+	 * (supportinfo is left as zeroes, and is filled on-the-fly when used)
+	 */
 	IndexSupportInitialize(strategy, support,
 						   &relation->rd_uniqueindex,
 						   RelationGetRelid(relation),
 						   relation->rd_rel->relam,
 						   amstrategies, amsupport, natts);
 
-	RelationSetIndexSupport(relation, strategy, support);
+	relation->rd_istrat = strategy;
+	relation->rd_support = support;
+	relation->rd_supportinfo = supportinfo;
 }
 
 /*
@@ -1595,11 +1628,9 @@ RelationClearRelation(Relation relation, bool rebuildIt)
 		pfree(relation->rd_am);
 	if (relation->rd_rel)
 		pfree(relation->rd_rel);
-	if (relation->rd_istrat)
-		pfree(relation->rd_istrat);
-	if (relation->rd_support)
-		pfree(relation->rd_support);
 	freeList(relation->rd_indexlist);
+	if (relation->rd_indexcxt)
+		MemoryContextDelete(relation->rd_indexcxt);
 
 	/*
 	 * If we're really done with the relcache entry, blow it away. But if
@@ -2624,8 +2655,11 @@ init_irels(void)
 	Relation	ird;
 	Form_pg_am	am;
 	Form_pg_class relform;
+	MemoryContext indexcxt;
 	IndexStrategy strat;
 	RegProcedure *support;
+	int			nstrategies,
+				nsupport;
 	int			i;
 	int			relno;
 
@@ -2646,6 +2680,13 @@ init_irels(void)
 			return;
 		}
 
+		/* safety check for incompatible relcache layout */
+		if (len != sizeof(RelationData))
+		{
+			write_irels();
+			return;
+		}
+
 		ird = irel[relno] = (Relation) palloc(len);
 		MemSet(ird, 0, len);
 
@@ -2659,6 +2700,7 @@ init_irels(void)
 		/* reset transient fields */
 		ird->rd_targblock = InvalidBlockNumber;
 		ird->rd_fd = -1;
+		ird->rd_refcnt = 0;
 
 		ird->rd_node.tblNode = MyDatabaseId;
 
@@ -2716,6 +2758,17 @@ init_irels(void)
 			}
 		}
 
+		/*
+		 * prepare index info context --- parameters should match
+		 * RelationInitIndexAccessInfo
+		 */
+		indexcxt = AllocSetContextCreate(CacheMemoryContext,
+										 RelationGetRelationName(ird),
+										 0,			/* minsize */
+										 512,		/* initsize */
+										 1024);		/* maxsize */
+		ird->rd_indexcxt = indexcxt;
+
 		/* next, read the index strategy map */
 		if ((nread = FileRead(fd, (char *) &len, sizeof(len))) != sizeof(len))
 		{
@@ -2723,27 +2776,18 @@ init_irels(void)
 			return;
 		}
 
-		strat = (IndexStrategy) palloc(len);
+		strat = (IndexStrategy) MemoryContextAlloc(indexcxt, len);
 		if ((nread = FileRead(fd, (char *) strat, len)) != len)
 		{
 			write_irels();
 			return;
 		}
 
-		/* oh, for god's sake... */
-#define SMD(i)	strat->strategyMapData[i].entry[0]
-
-		/* have to reinit the function pointers in the strategy maps */
-		for (i = 0; i < am->amstrategies * relform->relnatts; i++)
-		{
-			fmgr_info(SMD(i).sk_procedure,
-					  &(SMD(i).sk_func));
-		}
+		/* have to invalidate any FmgrInfo data in the strategy maps */
+		nstrategies = am->amstrategies * relform->relnatts;
+		for (i = 0; i < nstrategies; i++)
+			strat->strategyMapData[i].entry[0].sk_func.fn_oid = InvalidOid;
 
-		/*
-		 * use a real field called rd_istrat instead of the bogosity of
-		 * hanging invisible fields off the end of a structure - jolly
-		 */
 		ird->rd_istrat = strat;
 
 		/* finally, read the vector of support procedures */
@@ -2753,7 +2797,7 @@ init_irels(void)
 			return;
 		}
 
-		support = (RegProcedure *) palloc(len);
+		support = (RegProcedure *) MemoryContextAlloc(indexcxt, len);
 		if ((nread = FileRead(fd, (char *) support, len)) != len)
 		{
 			write_irels();
@@ -2761,6 +2805,11 @@ init_irels(void)
 		}
 		ird->rd_support = support;
 
+		nsupport = relform->relnatts * am->amsupport;
+		ird->rd_supportinfo = (FmgrInfo *)
+			MemoryContextAlloc(indexcxt, nsupport * sizeof(FmgrInfo));
+		MemSet(ird->rd_supportinfo, 0, nsupport * sizeof(FmgrInfo));
+
 		RelationInitLockInfo(ird);
 
 		RelationCacheInsert(ird);