Subject: ResourceOwner refactoring

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 17 Nov 2020 16:21:29 +0200

Two recent patches that I have reviewed have reminded me that the 
ResourceOwner interface is a bit clunky. There are two issues:

1. Performance. It's fast enough in most use, but when I was testing 
Kyotaro's catcache patches [1], the Resowner functions to track catcache 
references nevertheless showed up, accounting for about 20% of the CPU 
time used by catcache lookups.

2. It's difficult for extensions to use. There is a callback mechanism 
for extensions, but it's much less convenient to use than the built-in 
functions. The pgcrypto code uses the callbacks currently, and Michael's 
patch [2] would move that support for tracking OpenSSL contexts to the 
core, which makes it a lot more convenient for pgcrypto. Wouldn't it be 
nice if extensions could have the same ergonomics as built-in code, if 
they need to track external resources?

Attached patch refactors the ResourceOwner internals to do that.

The current code in HEAD has a separate array for each "kind" of object 
that can be tracked. The number of different kinds of objects has grown 
over the years, currently there is support for tracking buffers, files, 
catcache, relcache and plancache references, tupledescs, snapshots, DSM 
segments and LLVM JIT contexts. And locks, which are a bit special.

In the patch, I'm using the same array to hold all kinds of objects, and 
carry another field with each tracked object, to tell what kind of an 
object it is. An extension can define a new object kind, by defining 
Release and PrintLeakWarning callback functions for it. The code in 
resowner.c is now much more generic, as it doesn't need to know about 
all the different object kinds anymore (with a couple of exceptions). In 
the new scheme, there is one small fixed-size array to hold a few most 
recently-added references, that is linear-searched, and older entries 
are moved to a hash table.

I haven't done thorough performance testing of this, but with some quick 
testing with Kyotaro's "catcachebench" to stress-test syscache lookups, 
this performs a little better. In that test, all the activity happens in 
the small array portion, but I believe that's true for most use cases.

Thoughts? Can anyone suggest test scenarios to verify the performance of 
this?

[1] 
https://www.postgresql.org/message-id/20201106.172958.1103727352904717607.horikyota.ntt%40gmail.com

[2] https://www.postgresql.org/message-id/[email-masked]

- Heikki
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 30c30cf3a2e..c099f04f551 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -29,9 +29,21 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/syscache.h"
 
+/* ResourceOwner callbacks to hold tupledesc references  */
+static void ResOwnerReleaseTupleDesc(Datum res);
+static void ResOwnerPrintTupleDescLeakWarning(Datum res);
+
+static ResourceOwnerFuncs tupdesc_resowner_funcs =
+{
+	/* relcache references */
+	.name = "tupdesc reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseTupleDesc,
+	.PrintLeakWarning = ResOwnerPrintTupleDescLeakWarning
+};
 
 /*
  * CreateTemplateTupleDesc
@@ -376,9 +388,10 @@ IncrTupleDescRefCount(TupleDesc tupdesc)
 {
 	Assert(tupdesc->tdrefcount >= 0);
 
-	ResourceOwnerEnlargeTupleDescs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	tupdesc->tdrefcount++;
-	ResourceOwnerRememberTupleDesc(CurrentResourceOwner, tupdesc);
+	ResourceOwnerRemember(CurrentResourceOwner, PointerGetDatum(tupdesc),
+						  &tupdesc_resowner_funcs);
 }
 
 /*
@@ -394,7 +407,8 @@ DecrTupleDescRefCount(TupleDesc tupdesc)
 {
 	Assert(tupdesc->tdrefcount > 0);
 
-	ResourceOwnerForgetTupleDesc(CurrentResourceOwner, tupdesc);
+	ResourceOwnerForget(CurrentResourceOwner, PointerGetDatum(tupdesc),
+						&tupdesc_resowner_funcs);
 	if (--tupdesc->tdrefcount == 0)
 		FreeTupleDesc(tupdesc);
 }
@@ -925,3 +939,23 @@ BuildDescFromLists(List *names, List *types, List *typmods, List *collations)
 
 	return desc;
 }
+
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseTupleDesc(Datum res)
+{
+	DecrTupleDescRefCount((TupleDesc) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintTupleDescLeakWarning(Datum res)
+{
+	TupleDesc tupdesc = (TupleDesc) DatumGetPointer(res);
+
+	elog(WARNING,
+		 "TupleDesc reference leak: TupleDesc %p (%u,%d) still referenced",
+		 tupdesc, tupdesc->tdtypeid, tupdesc->tdtypmod);
+}
diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c
index 5ca3f922fed..c44080b9742 100644
--- a/src/backend/jit/jit.c
+++ b/src/backend/jit/jit.c
@@ -26,7 +26,6 @@
 #include "jit/jit.h"
 #include "miscadmin.h"
 #include "utils/fmgrprotos.h"
-#include "utils/resowner_private.h"
 
 /* GUCs */
 bool		jit_enabled = true;
@@ -140,7 +139,6 @@ jit_release_context(JitContext *context)
 	if (provider_successfully_loaded)
 		provider.release_context(context);
 
-	ResourceOwnerForgetJIT(context->resowner, PointerGetDatum(context));
 	pfree(context);
 }
 
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 40a439326c6..edea0388c04 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -40,7 +40,7 @@
 #include "portability/instr_time.h"
 #include "storage/ipc.h"
 #include "utils/memutils.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 
 /* Handle of a module emitted via ORC JIT */
 typedef struct LLVMJitHandle
@@ -121,8 +121,20 @@ static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm);
 static char *llvm_error_message(LLVMErrorRef error);
 #endif							/* LLVM_VERSION_MAJOR > 11 */
 
-PG_MODULE_MAGIC;
+/* ResourceOwner callbacks to hold JitContexts  */
+static void ResOwnerReleaseJitContext(Datum res);
+static void ResOwnerPrintJitContextLeakWarning(Datum res);
+
+static ResourceOwnerFuncs jit_funcs =
+{
+	/* relcache references */
+	.name = "LLVM JIT context",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseJitContext,
+	.PrintLeakWarning = ResOwnerPrintJitContextLeakWarning
+};
 
+PG_MODULE_MAGIC;
 
 /*
  * Initialize LLVM JIT provider.
@@ -151,7 +163,7 @@ llvm_create_context(int jitFlags)
 
 	llvm_session_initialize();
 
-	ResourceOwnerEnlargeJIT(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	context = MemoryContextAllocZero(TopMemoryContext,
 									 sizeof(LLVMJitContext));
@@ -159,7 +171,7 @@ llvm_create_context(int jitFlags)
 
 	/* ensure cleanup */
 	context->base.resowner = CurrentResourceOwner;
-	ResourceOwnerRememberJIT(CurrentResourceOwner, PointerGetDatum(context));
+	ResourceOwnerRemember(CurrentResourceOwner, PointerGetDatum(context), &jit_funcs);
 
 	return context;
 }
@@ -221,6 +233,8 @@ llvm_release_context(JitContext *context)
 
 		pfree(jit_handle);
 	}
+
+	ResourceOwnerForget(context->resowner, PointerGetDatum(context), &jit_funcs);
 }
 
 /*
@@ -1210,3 +1224,21 @@ llvm_error_message(LLVMErrorRef error)
 }
 
 #endif							/* LLVM_VERSION_MAJOR > 11 */
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseJitContext(Datum res)
+{
+	jit_release_context((JitContext *) PointerGetDatum(res));
+}
+
+static void
+ResOwnerPrintJitContextLeakWarning(Datum res)
+{
+	/* XXX: We used to not print these. Was that intentional? */
+	JitContext *context = (JitContext *) PointerGetDatum(res);
+
+	elog(WARNING, "JIT context leak: context %p still referenced", context);
+}
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ad0d1a9abc0..11a42cbdea6 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -52,7 +52,7 @@
 #include "utils/memdebug.h"
 #include "utils/ps_status.h"
 #include "utils/rel.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/timestamp.h"

@@ -198,6 +198,18 @@ static PrivateRefCountEntry *GetPrivateRefCountEntry(Buffer buffer, bool do_move
 static inline int32 GetPrivateRefCount(Buffer buffer);
 static void ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref);
 
+/* ResourceOwner callbacks to hold buffer pins */
+static void ResOwnerReleaseBuffer(Datum res);
+static void ResOwnerPrintBufferLeakWarning(Datum res);
+
+ResourceOwnerFuncs buffer_resowner_funcs =
+{
+	.name = "buffer",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseBuffer,
+	.PrintLeakWarning = ResOwnerPrintBufferLeakWarning
+};
+
 /*
  * Ensure that the PrivateRefCountArray has sufficient space to store one more
  * entry. This has to be called before using NewPrivateRefCountEntry() to fill
@@ -727,7 +739,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	*hit = false;
 
 	/* Make sure we will have room to remember the buffer pin */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	isExtend = (blockNum == P_NEW);
 
@@ -1846,7 +1858,7 @@ BufferSync(int flags)
 	WritebackContext wb_context;
 
 	/* Make sure we can handle the pin inside SyncOneBuffer */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	/*
 	 * Unless this is a shutdown checkpoint or we have been explicitly told,
@@ -2323,7 +2335,7 @@ BgBufferSync(WritebackContext *wb_context)
 	 */
 
 	/* Make sure we can handle the pin inside SyncOneBuffer */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	num_to_scan = bufs_to_lap;
 	num_written = 0;
@@ -3301,7 +3313,7 @@ FlushRelationBuffers(Relation rel)
 	}
 
 	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	for (i = 0; i < NBuffers; i++)
 	{
@@ -3374,7 +3386,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		pg_qsort(srels, nrels, sizeof(SMgrSortArray), rnode_comparator);
 
 	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	for (i = 0; i < NBuffers; i++)
 	{
@@ -3453,7 +3465,7 @@ FlushDatabaseBuffers(Oid dbid)
 	BufferDesc *bufHdr;
 
 	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	for (i = 0; i < NBuffers; i++)
 	{
@@ -3551,7 +3563,7 @@ void
 IncrBufferRefCount(Buffer buffer)
 {
 	Assert(BufferIsPinned(buffer));
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	if (BufferIsLocal(buffer))
 		LocalRefCount[-buffer - 1]++;
 	else
@@ -4585,3 +4597,18 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
 				(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
 				 errmsg("snapshot too old")));
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseBuffer(Datum res)
+{
+	ReleaseBuffer(DatumGetInt32(res));
+}
+
+static void
+ResOwnerPrintBufferLeakWarning(Datum res)
+{
+	PrintBufferLeakWarning(DatumGetInt32(res));
+}
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 6ffd7b33062..cbf9819cba5 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -22,7 +22,7 @@
 #include "storage/bufmgr.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"

 /*#define LBDEBUG*/
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 05abcf72d68..ba6262c83da 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -96,7 +96,7 @@
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "utils/guc.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 
 /* Define PG_FLUSH_DATA_WORKS if we have an implementation for pg_flush_data */
 #if defined(HAVE_SYNC_FILE_RANGE)
@@ -339,6 +339,24 @@ static void unlink_if_exists_fname(const char *fname, bool isdir, int elevel);
 static int	fsync_parent_path(const char *fname, int elevel);

+/* ResourceOwner callbacks to hold virtual file descriptors */
+static void ResOwnerReleaseFile(Datum res);
+static void ResOwnerPrintFileLeakWarning(Datum res);
+
+static ResourceOwnerFuncs file_resowner_funcs =
+{
+	.name = "File",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseFile,
+	.PrintLeakWarning = ResOwnerPrintFileLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberFile(owner, file) \
+	ResourceOwnerRemember(owner, Int32GetDatum(file), &file_resowner_funcs)
+#define ResourceOwnerForgetFile(owner, file) \
+	ResourceOwnerForget(owner, Int32GetDatum(file), &file_resowner_funcs)
+
 /*
  * pg_fsync --- do fsync with or without writethrough
  */
@@ -1402,7 +1420,7 @@ ReportTemporaryFileUsage(const char *path, off_t size)
 
 /*
  * Called to register a temporary file for automatic close.
- * ResourceOwnerEnlargeFiles(CurrentResourceOwner) must have been called
+ * ResourceOwnerEnlarge(CurrentResourceOwner) must have been called
  * before the file was opened.
  */
 static void
@@ -1584,7 +1602,7 @@ OpenTemporaryFile(bool interXact)
 	 * open it, if we'll be registering it below.
 	 */
 	if (!interXact)
-		ResourceOwnerEnlargeFiles(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	/*
 	 * If some temp tablespace(s) have been given to us, try to use the next
@@ -1714,7 +1732,7 @@ PathNameCreateTemporaryFile(const char *path, bool error_on_failure)
 {
 	File		file;
 
-	ResourceOwnerEnlargeFiles(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	/*
 	 * Open the file.  Note: we don't use O_EXCL, in case there is an orphaned
@@ -1752,7 +1770,7 @@ PathNameOpenTemporaryFile(const char *path, int mode)
 {
 	File		file;
 
-	ResourceOwnerEnlargeFiles(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	file = PathNameOpenFile(path, mode | PG_BINARY);
 
@@ -3604,3 +3622,19 @@ data_sync_elevel(int elevel)
 {
 	return data_sync_retry ? elevel : PANIC;
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseFile(Datum res)
+{
+	FileClose((File) DatumGetInt32(res));
+}
+
+static void
+ResOwnerPrintFileLeakWarning(Datum res)
+{
+	elog(WARNING, "temporary file leak: File %d still referenced",
+		 DatumGetInt32(res));
+}
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index dffbd8e82a2..c665bc6cecf 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -37,13 +37,15 @@
 #include "miscadmin.h"
 #include "port/pg_bitutils.h"
 #include "storage/dsm.h"
+#include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/lwlock.h"
 #include "storage/pg_shmem.h"
+#include "storage/shmem.h"
 #include "utils/freepage.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 
 #define PG_DYNSHMEM_CONTROL_MAGIC		0x9a503d32
 
@@ -139,6 +141,25 @@ static dsm_control_header *dsm_control;
 static Size dsm_control_mapped_size = 0;
 static void *dsm_control_impl_private = NULL;
 
+
+/* ResourceOwner callbacks to hold DSM segments */
+static void ResOwnerReleaseDSM(Datum res);
+static void ResOwnerPrintDSMLeakWarning(Datum res);
+
+static ResourceOwnerFuncs dsm_resowner_funcs =
+{
+	.name = "dynamic shared memory segment",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseDSM,
+	.PrintLeakWarning = ResOwnerPrintDSMLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberDSM(owner, seg) \
+	ResourceOwnerRemember(owner, PointerGetDatum(seg), &dsm_resowner_funcs)
+#define ResourceOwnerForgetDSM(owner, seg) \
+	ResourceOwnerForget(owner, PointerGetDatum(seg), &dsm_resowner_funcs)
+
 /*
  * Start up the dynamic shared memory system.
  *
@@ -895,7 +916,7 @@ void
 dsm_unpin_mapping(dsm_segment *seg)
 {
 	Assert(seg->resowner == NULL);
-	ResourceOwnerEnlargeDSMs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	seg->resowner = CurrentResourceOwner;
 	ResourceOwnerRememberDSM(seg->resowner, seg);
 }
@@ -1162,7 +1183,7 @@ dsm_create_descriptor(void)
 	dsm_segment *seg;
 
 	if (CurrentResourceOwner)
-		ResourceOwnerEnlargeDSMs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	seg = MemoryContextAlloc(TopMemoryContext, sizeof(dsm_segment));
 	dlist_push_head(&dsm_segment_list, &seg->node);
@@ -1241,3 +1262,20 @@ is_main_region_dsm_handle(dsm_handle handle)
 {
 	return handle & 1;
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseDSM(Datum res)
+{
+	dsm_detach((dsm_segment *) DatumGetPointer(res));
+}
+static void
+ResOwnerPrintDSMLeakWarning(Datum res)
+{
+	dsm_segment *seg = (dsm_segment *) res;
+
+	elog(WARNING, "dynamic shared memory leak: segment %u still referenced",
+		 dsm_segment_handle(seg));
+}
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index d86566f4554..98b4725c406 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -47,7 +47,7 @@
 #include "storage/standby.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"

 /* This configuration variable is used to set the lock table size */
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 3613ae5f44d..c9ebf9effe1 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -31,12 +31,13 @@
 #endif
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
+#include "utils/catcache.h"
 #include "utils/datum.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/syscache.h"

@@ -104,6 +105,66 @@ static void CatCacheCopyKeys(TupleDesc tupdesc, int nkeys, int *attnos,
  *					internal support functions
  */
 
+/* ResourceOwner callbacks to hold catcache references */
+
+static void ResOwnerReleaseCatCache(Datum res);
+static void ResOwnerPrintCatCacheLeakWarning(Datum res);
+static void ResOwnerReleaseCatCacheList(Datum res);
+static void ResOwnerPrintCatCacheListLeakWarning(Datum res);
+
+static ResourceOwnerFuncs catcache_funcs =
+{
+	/* catcache references */
+	.name = "catcache reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCatCache,
+	.PrintLeakWarning = ResOwnerPrintCatCacheLeakWarning
+};
+
+static ResourceOwnerFuncs catlistref_funcs =
+{
+	/* catcache-list pins */
+	.name = "catcache list reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCatCacheList,
+	.PrintLeakWarning = ResOwnerPrintCatCacheListLeakWarning
+};
+
+/* support for catcache refcount management */
+static inline void
+ResourceOwnerEnlargeCatCacheRefs(ResourceOwner owner)
+{
+	ResourceOwnerEnlarge(owner);
+}
+static inline void
+ResourceOwnerRememberCatCacheRef(ResourceOwner owner, HeapTuple tuple)
+{
+	ResourceOwnerRemember(owner, PointerGetDatum(tuple), &catcache_funcs);
+}
+static inline void
+ResourceOwnerForgetCatCacheRef(ResourceOwner owner, HeapTuple tuple)
+{
+	ResourceOwnerForget(owner, PointerGetDatum(tuple), &catcache_funcs);
+}
+
+static inline void
+ResourceOwnerEnlargeCatCacheListRefs(ResourceOwner owner)
+{
+	ResourceOwnerEnlarge(owner);
+}
+static inline void
+ResourceOwnerRememberCatCacheListRef(ResourceOwner owner, CatCList *list)
+{
+	ResourceOwnerRemember(owner, PointerGetDatum(list), &catlistref_funcs);
+}
+
+static inline void
+ResourceOwnerForgetCatCacheListRef(ResourceOwner owner, CatCList *list)
+{
+	ResourceOwnerForget(owner, PointerGetDatum(list), &catlistref_funcs);
+}
+
+
 /*
  * Hash and equality functions for system types that are used as cache key
  * fields.  In some cases, we just call the regular SQL-callable functions for
@@ -1270,7 +1331,7 @@ SearchCatCacheInternal(CatCache *cache,
 		 */
 		if (!ct->negative)
 		{
-			ResourceOwnerEnlargeCatCacheRefs(CurrentResourceOwner);
+			ResourceOwnerEnlarge(CurrentResourceOwner);
 			ct->refcount++;
 			ResourceOwnerRememberCatCacheRef(CurrentResourceOwner, &ct->tuple);
 
@@ -1371,7 +1432,7 @@ SearchCatCacheMiss(CatCache *cache,
 									 hashValue, hashIndex,
 									 false);
 		/* immediately set the refcount to 1 */
-		ResourceOwnerEnlargeCatCacheRefs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 		ct->refcount++;
 		ResourceOwnerRememberCatCacheRef(CurrentResourceOwner, &ct->tuple);
 		break;					/* assume only one match */
@@ -1583,7 +1644,7 @@ SearchCatCacheList(CatCache *cache,
 		dlist_move_head(&cache->cc_lists, &cl->cache_elem);
 
 		/* Bump the list's refcount and return it */
-		ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 		cl->refcount++;
 		ResourceOwnerRememberCatCacheListRef(CurrentResourceOwner, cl);
 
@@ -1607,7 +1668,7 @@ SearchCatCacheList(CatCache *cache,
 	 * block to ensure we can undo those refcounts if we get an error before
 	 * we finish constructing the CatCList.
 	 */
-	ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	ctlist = NIL;
 
@@ -2062,14 +2123,19 @@ PrepareToInvalidateCacheTuple(Relation relation,
 	}
 }
 
-
 /*
- * Subroutines for warning about reference leaks.  These are exported so
- * that resowner.c can call them.
+ * ResourceOwner callbacks
  */
-void
-PrintCatCacheLeakWarning(HeapTuple tuple)
+static void
+ResOwnerReleaseCatCache(Datum res)
 {
+	ReleaseCatCache((HeapTuple) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintCatCacheLeakWarning(Datum res)
+{
+	HeapTuple tuple = (HeapTuple) DatumGetPointer(res);
 	CatCTup    *ct = (CatCTup *) (((char *) tuple) -
 								  offsetof(CatCTup, tuple));
 
@@ -2083,9 +2149,17 @@ PrintCatCacheLeakWarning(HeapTuple tuple)
 		 ct->refcount);
 }
 
-void
-PrintCatCacheListLeakWarning(CatCList *list)
+static void
+ResOwnerReleaseCatCacheList(Datum res)
+{
+	ReleaseCatCacheList((CatCList *) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintCatCacheListLeakWarning(Datum res)
 {
+	CatCList *list = (CatCList *) DatumGetPointer(res);
+
 	elog(WARNING, "cache reference leak: cache %s (%d), list %p has count %d",
 		 list->my_cache->cc_relname, list->my_cache->id,
 		 list, list->refcount);
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 50d6ad28b4c..8bce9d16ed5 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -69,7 +69,7 @@
 #include "tcop/utility.h"
 #include "utils/inval.h"
 #include "utils/memutils.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/rls.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
@@ -115,6 +115,31 @@ static void PlanCacheRelCallback(Datum arg, Oid relid);
 static void PlanCacheObjectCallback(Datum arg, int cacheid, uint32 hashvalue);
 static void PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue);
 
+/* ResourceOwner callbacks to track plancache references */
+static void ResOwnerReleaseCachedPlan(Datum res);
+static void ResOwnerPrintPlanCacheLeakWarning(Datum res);
+
+/* this is exported for ResourceOwnerReleaseAllPlanCacheRefs() */
+ResourceOwnerFuncs planref_resowner_funcs =
+{
+	.name = "plancache reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCachedPlan,
+	.PrintLeakWarning = ResOwnerPrintPlanCacheLeakWarning
+};
+
+static inline void
+ResourceOwnerRememberPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
+{
+	ResourceOwnerRemember(owner, PointerGetDatum(plan), &planref_resowner_funcs);
+}
+static inline void
+ResourceOwnerForgetPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
+{
+	ResourceOwnerForget(owner, PointerGetDatum(plan), &planref_resowner_funcs);
+}
+
+
 /* GUC parameter */
 int			plan_cache_mode;
 
@@ -1229,7 +1254,7 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 
 	/* Flag the plan as in use by caller */
 	if (useResOwner)
-		ResourceOwnerEnlargePlanCacheRefs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 	plan->refcount++;
 	if (useResOwner)
 		ResourceOwnerRememberPlanCacheRef(CurrentResourceOwner, plan);
@@ -1392,7 +1417,7 @@ CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
 	/* Bump refcount if requested. */
 	if (owner)
 	{
-		ResourceOwnerEnlargePlanCacheRefs(owner);
+		ResourceOwnerEnlarge(owner);
 		plan->refcount++;
 		ResourceOwnerRememberPlanCacheRef(owner, plan);
 	}
@@ -1451,7 +1476,7 @@ CachedPlanIsSimplyValid(CachedPlanSource *plansource, CachedPlan *plan,
 	/* It's still good.  Bump refcount if requested. */
 	if (owner)
 	{
-		ResourceOwnerEnlargePlanCacheRefs(owner);
+		ResourceOwnerEnlarge(owner);
 		plan->refcount++;
 		ResourceOwnerRememberPlanCacheRef(owner, plan);
 	}
@@ -2205,3 +2230,20 @@ ResetPlanCache(void)
 		cexpr->is_valid = false;
 	}
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+
+static void
+ResOwnerReleaseCachedPlan(Datum res)
+{
+	ReleaseCachedPlan((CachedPlan *) DatumGetPointer(res), true);
+}
+
+static void
+ResOwnerPrintPlanCacheLeakWarning(Datum res)
+{
+	elog(WARNING, "plancache reference leak: plan %p not closed",
+		 DatumGetPointer(res));
+}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 66393becfb6..efff30e9238 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -78,13 +78,14 @@
 #include "storage/smgr.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
+#include "utils/catcache.h"
 #include "utils/datum.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/relmapper.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 
@@ -2066,6 +2067,18 @@ RelationIdGetRelation(Oid relationId)
  * ----------------------------------------------------------------
  */
 
+/* ResourceOwner callbacks to track relcache references */
+static void ResOwnerReleaseRelation(Datum res);
+static void ResOwnerPrintRelCacheLeakWarning(Datum res);
+
+static ResourceOwnerFuncs relref_resowner_funcs =
+{
+	.name = "relcache reference",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseRelation,
+	.PrintLeakWarning = ResOwnerPrintRelCacheLeakWarning
+};
+
 /*
  * RelationIncrementReferenceCount
  *		Increments relation reference count.
@@ -2077,10 +2090,10 @@ RelationIdGetRelation(Oid relationId)
 void
 RelationIncrementReferenceCount(Relation rel)
 {
-	ResourceOwnerEnlargeRelationRefs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	rel->rd_refcnt += 1;
 	if (!IsBootstrapProcessingMode())
-		ResourceOwnerRememberRelationRef(CurrentResourceOwner, rel);
+		ResourceOwnerRemember(CurrentResourceOwner, PointerGetDatum(rel), &relref_resowner_funcs);
 }
 
 /*
@@ -2093,7 +2106,7 @@ RelationDecrementReferenceCount(Relation rel)
 	Assert(rel->rd_refcnt > 0);
 	rel->rd_refcnt -= 1;
 	if (!IsBootstrapProcessingMode())
-		ResourceOwnerForgetRelationRef(CurrentResourceOwner, rel);
+		ResourceOwnerForget(CurrentResourceOwner, PointerGetDatum(rel), &relref_resowner_funcs);
 }
 
 /*
@@ -6406,3 +6419,21 @@ unlink_initfile(const char *initfilename, int elevel)
 							initfilename)));
 	}
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerPrintRelCacheLeakWarning(Datum res)
+{
+	Relation rel = (Relation) res;
+
+	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
+		 RelationGetRelationName(rel));
+}
+
+static void
+ResOwnerReleaseRelation(Datum res)
+{
+	RelationClose((Relation) res);
+}
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 809b27a038f..a3c4063e655 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -982,7 +982,7 @@ static const struct cachedesc cacheinfo[] = {
 	}
 };
 
-static CatCache *SysCache[SysCacheSize];
+ CatCache *SysCache[SysCacheSize];
 
 static bool CacheInitialized = false;
 
diff --git a/src/backend/utils/resowner/README b/src/backend/utils/resowner/README
index 2998f6bb362..890db7d1e66 100644
--- a/src/backend/utils/resowner/README
+++ b/src/backend/utils/resowner/README
@@ -60,13 +60,18 @@ subtransaction or portal.  Therefore, the "release" operation on a child
 ResourceOwner transfers lock ownership to the parent instead of actually
 releasing the lock, if isCommit is true.
 
-Currently, ResourceOwners contain direct support for recording ownership of
-buffer pins, lmgr locks, and catcache, relcache, plancache, tupdesc, and
-snapshot references.  Other objects can be associated with a ResourceOwner by
-recording the address of the owning ResourceOwner in such an object.  There is
-an API for other modules to get control during ResourceOwner release, so that
-they can scan their own data structures to find the objects that need to be
-deleted.
+ResourceOwner can record ownership of many different kinds of objects.
+As of this writing, it's used internally for buffer pins, lmgr locks, and
+catcache, relcache, plancache, tupdesc, snapshot, DSM and JIT context references.
+ResourceOwner treates all objects the same, but to register a new kind of
+an object with it, you need to fill in a few callback functions, see
+ResourceOwnerFuncs.
+
+There is also an API for other modules to get control during ResourceOwner
+release, so that they can scan their own data structures to find the objects
+that need to be deleted. This used to be the only way to register new kinds
+of objects with a resource owner; nowadays it easier to write custom
+ResourceOwnerFuncs callabacks.
 
 Whenever we are inside a transaction, the global variable
 CurrentResourceOwner shows which resource owner should be assigned
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 8bc2c4e9ea3..2e1ae4f8e0c 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -21,73 +21,44 @@
 #include "postgres.h"
 
 #include "common/hashfn.h"
-#include "jit/jit.h"
-#include "storage/bufmgr.h"
 #include "storage/ipc.h"
 #include "storage/predicate.h"
 #include "storage/proc.h"
 #include "utils/memutils.h"
-#include "utils/rel.h"
-#include "utils/resowner_private.h"
-#include "utils/snapmgr.h"
-
+#include "utils/plancache.h"
+#include "utils/resowner.h"
 
 /*
- * All resource IDs managed by this code are required to fit into a Datum,
- * which is fine since they are generally pointers or integers.
- *
- * Provide Datum conversion macros for a couple of things that are really
- * just "int".
- */
-#define FileGetDatum(file) Int32GetDatum(file)
-#define DatumGetFile(datum) ((File) DatumGetInt32(datum))
-#define BufferGetDatum(buffer) Int32GetDatum(buffer)
-#define DatumGetBuffer(datum) ((Buffer) DatumGetInt32(datum))
-
-/*
- * ResourceArray is a common structure for storing all types of resource IDs.
- *
- * We manage small sets of resource IDs by keeping them in a simple array:
- * itemsarr[k] holds an ID, for 0 <= k < nitems <= maxitems = capacity.
+ * ResourceElem represents a reference associated with a resource owner.
  *
- * If a set grows large, we switch over to using open-addressing hashing.
- * Then, itemsarr[] is a hash table of "capacity" slots, with each
- * slot holding either an ID or "invalidval".  nitems is the number of valid
- * items present; if it would exceed maxitems, we enlarge the array and
- * re-hash.  In this mode, maxitems should be rather less than capacity so
- * that we don't waste too much time searching for empty slots.
- *
- * In either mode, lastidx remembers the location of the last item inserted
- * or returned by GetAny; this speeds up searches in ResourceArrayRemove.
+ * All objects managed by this code are required to fit into a Datum,
+ * which is fine since they are generally pointers or integers.
  */
-typedef struct ResourceArray
+typedef struct ResourceElem
 {
-	Datum	   *itemsarr;		/* buffer for storing values */
-	Datum		invalidval;		/* value that is considered invalid */
-	uint32		capacity;		/* allocated length of itemsarr[] */
-	uint32		nitems;			/* how many items are stored in items array */
-	uint32		maxitems;		/* current limit on nitems before enlarging */
-	uint32		lastidx;		/* index of last item returned by GetAny */
-} ResourceArray;
+	Datum		item;
+	ResourceOwnerFuncs *kind;
+} ResourceElem;
 
 /*
- * Initially allocated size of a ResourceArray.  Must be power of two since
- * we'll use (arraysize - 1) as mask for hashing.
+ * Size of the small fixed-size array to hold most-recently remembered resources.
  */
-#define RESARRAY_INIT_SIZE 16
+#define RESOWNER_ARRAY_SIZE 8
 
 /*
- * When to switch to hashing vs. simple array logic in a ResourceArray.
+ * Initially allocated size of a ResourceOwner's hash.  Must be power of two since
+ * we'll use (capacity - 1) as mask for hashing.
  */
-#define RESARRAY_MAX_ARRAY 64
-#define RESARRAY_IS_ARRAY(resarr) ((resarr)->capacity <= RESARRAY_MAX_ARRAY)
+#define RESOWNER_HASH_INIT_SIZE 32
 
 /*
- * How many items may be stored in a resource array of given capacity.
+ * How many items may be stored in a hash of given capacity.
  * When this number is reached, we must resize.
  */
-#define RESARRAY_MAX_ITEMS(capacity) \
-	((capacity) <= RESARRAY_MAX_ARRAY ? (capacity) : (capacity)/4 * 3)
+#define RESOWNER_HASH_MAX_ITEMS(capacity) ((capacity)/4 * 3)
+
+StaticAssertDecl(RESOWNER_HASH_MAX_ITEMS(RESOWNER_HASH_INIT_SIZE) > RESOWNER_ARRAY_SIZE,
+				 "initial hash size too small compared to array size");
 
 /*
  * To speed up bulk releasing or reassigning locks from a resource owner to
@@ -117,22 +88,33 @@ typedef struct ResourceOwnerData
 	ResourceOwner nextchild;	/* next child of same parent */
 	const char *name;			/* name (just for debugging) */
 
-	/* We have built-in support for remembering: */
-	ResourceArray bufferarr;	/* owned buffers */
-	ResourceArray catrefarr;	/* catcache references */
-	ResourceArray catlistrefarr;	/* catcache-list pins */
-	ResourceArray relrefarr;	/* relcache references */
-	ResourceArray planrefarr;	/* plancache references */
-	ResourceArray tupdescarr;	/* tupdesc references */
-	ResourceArray snapshotarr;	/* snapshot references */
-	ResourceArray filearr;		/* open temporary files */
-	ResourceArray dsmarr;		/* dynamic shmem segments */
-	ResourceArray jitarr;		/* JIT contexts */
+	/*
+	 * These structs keep track of the objects registered with this owner.
+	 *
+	 * We manage a small set of references by keeping them in a simple
+	 * array. When the array gets full, all the elements in the array are
+	 * moved to a hash table. This way, the array always contains a few
+	 * most recently remembered references. To find a particular reference,
+	 * you need to search both the array and the hash table.
+	 */
+	ResourceElem arr[RESOWNER_ARRAY_SIZE];
+	uint32		narr;			/* how many items are stored in the array */
+
+	/*
+	 * The hash table. Uses open-addressing. 'nhash' is the number of items
+	 * present; if it would exceed 'grow_at', we enlarge it and re-hash.
+	 * 'grow_at' should be rather less than 'capacity' so that we don't waste
+	 * too much time searching for empty slots.
+	 */
+	ResourceElem *hash;
+	uint32		nhash;			/* how many items are stored in the hash */
+	uint32		capacity;		/* allocated length of hash[] */
+	uint32		grow_at;		/* grow hash when reach this */
 
 	/* We can remember up to MAX_RESOWNER_LOCKS references to local locks. */
 	int			nlocks;			/* number of owned locks */
 	LOCALLOCK  *locks[MAX_RESOWNER_LOCKS];	/* list of owned locks */
-}			ResourceOwnerData;
+} ResourceOwnerData;

 /*****************************************************************************
@@ -144,6 +126,18 @@ ResourceOwner CurTransactionResourceOwner = NULL;
 ResourceOwner TopTransactionResourceOwner = NULL;
 ResourceOwner AuxProcessResourceOwner = NULL;
 
+/* #define RESOWNER_STATS */
+/* #define RESOWNER_TRACE */
+
+#ifdef RESOWNER_STATS
+static int narray_lookups = 0;
+static int nhash_lookups = 0;
+#endif
+
+#ifdef RESOWNER_TRACE
+static int resowner_trace_counter = 0;
+#endif
+
 /*
  * List of add-on callbacks for resource releasing
  */
@@ -158,44 +152,36 @@ static ResourceReleaseCallbackItem *ResourceRelease_callbacks = NULL;

 /* Internal routines */
-static void ResourceArrayInit(ResourceArray *resarr, Datum invalidval);
-static void ResourceArrayEnlarge(ResourceArray *resarr);
-static void ResourceArrayAdd(ResourceArray *resarr, Datum value);
-static bool ResourceArrayRemove(ResourceArray *resarr, Datum value);
-static bool ResourceArrayGetAny(ResourceArray *resarr, Datum *value);
-static void ResourceArrayFree(ResourceArray *resarr);
 static void ResourceOwnerReleaseInternal(ResourceOwner owner,
 										 ResourceReleasePhase phase,
 										 bool isCommit,
 										 bool isTopLevel);
 static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);
-static void PrintRelCacheLeakWarning(Relation rel);
-static void PrintPlanCacheLeakWarning(CachedPlan *plan);
-static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
-static void PrintSnapshotLeakWarning(Snapshot snapshot);
-static void PrintFileLeakWarning(File file);
-static void PrintDSMLeakWarning(dsm_segment *seg);
+
+

 /*****************************************************************************
  *	  INTERNAL ROUTINES														 *
  *****************************************************************************/
 
-
-/*
- * Initialize a ResourceArray
- */
 static void
-ResourceArrayInit(ResourceArray *resarr, Datum invalidval)
+ResourceArrayAddToHash(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 {
-	/* Assert it's empty */
-	Assert(resarr->itemsarr == NULL);
-	Assert(resarr->capacity == 0);
-	Assert(resarr->nitems == 0);
-	Assert(resarr->maxitems == 0);
-	/* Remember the appropriate "invalid" value */
-	resarr->invalidval = invalidval;
-	/* We don't allocate any storage until needed */
+	/* Insert into first free slot at or after hash location. */
+	uint32		mask = owner->capacity - 1;
+	uint32		idx;
+
+	idx = DatumGetUInt32(hash_any((void *) &value, sizeof(value))) & mask;
+	for (;;)
+	{
+		if (owner->hash[idx].kind == NULL)
+			break;
+		idx = (idx + 1) & mask;
+	}
+	owner->hash[idx].item = value;
+	owner->hash[idx].kind = kind;
+	owner->nhash++;
 }
 
 /*
@@ -204,205 +190,222 @@ ResourceArrayInit(ResourceArray *resarr, Datum invalidval)
  * This is separate from actually inserting a resource because if we run out
  * of memory, it's critical to do so *before* acquiring the resource.
  */
-static void
-ResourceArrayEnlarge(ResourceArray *resarr)
+void
+ResourceOwnerEnlarge(ResourceOwner owner)
 {
-	uint32		i,
-				oldcap,
-				newcap;
-	Datum	   *olditemsarr;
-	Datum	   *newitemsarr;
-
-	if (resarr->nitems < resarr->maxitems)
+	if (owner->narr < RESOWNER_ARRAY_SIZE)
 		return;					/* no work needed */
 
-	olditemsarr = resarr->itemsarr;
-	oldcap = resarr->capacity;
+	/* Is there space in the hash? If not, enlarge it. */
 
 	/* Double the capacity of the array (capacity must stay a power of 2!) */
-	newcap = (oldcap > 0) ? oldcap * 2 : RESARRAY_INIT_SIZE;
-	newitemsarr = (Datum *) MemoryContextAlloc(TopMemoryContext,
-											   newcap * sizeof(Datum));
-	for (i = 0; i < newcap; i++)
-		newitemsarr[i] = resarr->invalidval;
-
-	/* We assume we can't fail below this point, so OK to scribble on resarr */
-	resarr->itemsarr = newitemsarr;
-	resarr->capacity = newcap;
-	resarr->maxitems = RESARRAY_MAX_ITEMS(newcap);
-	resarr->nitems = 0;
-
-	if (olditemsarr != NULL)
+	if (owner->narr + owner->nhash >= owner->grow_at)
 	{
-		/*
-		 * Transfer any pre-existing entries into the new array; they don't
-		 * necessarily go where they were before, so this simple logic is the
-		 * best way.  Note that if we were managing the set as a simple array,
-		 * the entries after nitems are garbage, but that shouldn't matter
-		 * because we won't get here unless nitems was equal to oldcap.
-		 */
-		for (i = 0; i < oldcap; i++)
+		uint32		i,
+					oldcap,
+					newcap;
+		ResourceElem *oldhash;
+		ResourceElem *newhash;
+
+		oldhash = owner->hash;
+		oldcap = owner->capacity;
+
+		newcap = (oldcap > 0) ? oldcap * 2 : RESOWNER_HASH_INIT_SIZE;
+		newhash = (ResourceElem *) MemoryContextAllocZero(TopMemoryContext,
+														  newcap * sizeof(ResourceElem));
+
+		/* We assume we can't fail below this point, so OK to scribble on FIXME */
+		owner->hash = newhash;
+		owner->capacity = newcap;
+		owner->grow_at = RESOWNER_HASH_MAX_ITEMS(newcap);
+		owner->nhash = 0;
+
+		if (oldhash != NULL)
 		{
-			if (olditemsarr[i] != resarr->invalidval)
-				ResourceArrayAdd(resarr, olditemsarr[i]);
+			/*
+			 * Transfer any pre-existing entries into the new array; they don't
+			 * necessarily go where they were before, so this simple logic is the
+			 * best way.
+			 */
+			for (i = 0; i < oldcap; i++)
+			{
+				if (oldhash[i].kind != NULL)
+					ResourceArrayAddToHash(owner, oldhash[i].item, oldhash[i].kind);
+			}
+
+			/* And release old array. */
+			pfree(oldhash);
 		}
+	}
 
-		/* And release old array. */
-		pfree(olditemsarr);
+	/* Move items from the array to the hash */
+	for (int i = 0; i < owner->narr; i++)
+	{
+		ResourceArrayAddToHash(owner, owner->arr[i].item, owner->arr[i].kind);
 	}
+	owner->narr = 0;
 
-	Assert(resarr->nitems < resarr->maxitems);
+	Assert(owner->nhash < owner->grow_at);
 }
 
 /*
- * Add a resource to ResourceArray
+ * Remember that an object is owner by a ReourceOwner
  *
- * Caller must have previously done ResourceArrayEnlarge()
+ * Caller must have previously done ResourceOwnerEnlarge()
  */
-static void
-ResourceArrayAdd(ResourceArray *resarr, Datum value)
+void
+ResourceOwnerRemember(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 {
 	uint32		idx;
 
-	Assert(value != resarr->invalidval);
-	Assert(resarr->nitems < resarr->maxitems);
+#ifdef RESOWNER_TRACE
+	elog(LOG, "REMEMBER %d: owner %p value " UINT64_FORMAT ", kind: %s",
+		 resowner_trace_counter++, owner, DatumGetUInt64(value), kind->name);
+#endif
 
-	if (RESARRAY_IS_ARRAY(resarr))
-	{
-		/* Append to linear array. */
-		idx = resarr->nitems;
-	}
-	else
-	{
-		/* Insert into first free slot at or after hash location. */
-		uint32		mask = resarr->capacity - 1;
+	Assert(owner->narr < RESOWNER_ARRAY_SIZE);
 
-		idx = DatumGetUInt32(hash_any((void *) &value, sizeof(value))) & mask;
-		for (;;)
-		{
-			if (resarr->itemsarr[idx] == resarr->invalidval)
-				break;
-			idx = (idx + 1) & mask;
-		}
-	}
-	resarr->lastidx = idx;
-	resarr->itemsarr[idx] = value;
-	resarr->nitems++;
+	/* Append to linear array. */
+	idx = owner->narr;
+	owner->arr[idx].item = value;
+	owner->arr[idx].kind = kind;
+	owner->narr++;
 }
 
 /*
- * Remove a resource from ResourceArray
+ * Forget that an object is owned by a ResourceOwner
  *
- * Returns true on success, false if resource was not found.
+ * Returns true on success. If the resource was not found, returns false,
+ * and calls kind->ForgetError callback.
  *
- * Note: if same resource ID appears more than once, one instance is removed.
+ * Note: if same resource ID is associated with the ResourceOwner more than once,
+ * one instance is removed.
  */
-static bool
-ResourceArrayRemove(ResourceArray *resarr, Datum value)
+void
+ResourceOwnerForget(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 {
 	uint32		i,
-				idx,
-				lastidx = resarr->lastidx;
+				idx;
 
-	Assert(value != resarr->invalidval);
+#ifdef RESOWNER_TRACE
+	elog(LOG, "FORGET %d: owner %p value " UINT64_FORMAT ", kind: %s",
+		 resowner_trace_counter++, owner, DatumGetUInt64(value), kind->name);
+#endif
 
-	/* Search through all items, but try lastidx first. */
-	if (RESARRAY_IS_ARRAY(resarr))
+	/* Search through all items, but check the array first. */
+	for (i = 0; i < owner->narr; i++)
 	{
-		if (lastidx < resarr->nitems &&
-			resarr->itemsarr[lastidx] == value)
-		{
-			resarr->itemsarr[lastidx] = resarr->itemsarr[resarr->nitems - 1];
-			resarr->nitems--;
-			/* Update lastidx to make reverse-order removals fast. */
-			resarr->lastidx = resarr->nitems - 1;
-			return true;
-		}
-		for (i = 0; i < resarr->nitems; i++)
+		if (owner->arr[i].item == value &&
+			owner->arr[i].kind == kind)
 		{
-			if (resarr->itemsarr[i] == value)
-			{
-				resarr->itemsarr[i] = resarr->itemsarr[resarr->nitems - 1];
-				resarr->nitems--;
-				/* Update lastidx to make reverse-order removals fast. */
-				resarr->lastidx = resarr->nitems - 1;
-				return true;
-			}
+			owner->arr[i] = owner->arr[owner->narr - 1];
+			owner->narr--;
+
+#ifdef RESOWNER_STATS
+			narray_lookups++;
+#endif
+
+			return;
 		}
 	}
-	else
+
+	/* Search hash */
+	if (owner->nhash > 0)
 	{
-		uint32		mask = resarr->capacity - 1;
+		uint32		mask = owner->capacity - 1;
 
-		if (lastidx < resarr->capacity &&
-			resarr->itemsarr[lastidx] == value)
-		{
-			resarr->itemsarr[lastidx] = resarr->invalidval;
-			resarr->nitems--;
-			return true;
-		}
 		idx = DatumGetUInt32(hash_any((void *) &value, sizeof(value))) & mask;
-		for (i = 0; i < resarr->capacity; i++)
+		for (i = 0; i < owner->capacity; i++)
 		{
-			if (resarr->itemsarr[idx] == value)
+			if (owner->hash[idx].item == value &&
+				owner->hash[idx].kind == kind)
 			{
-				resarr->itemsarr[idx] = resarr->invalidval;
-				resarr->nitems--;
-				return true;
+				owner->hash[idx].item = (Datum) 0;
+				owner->hash[idx].kind = NULL;
+				owner->nhash--;
+
+#ifdef RESOWNER_STATS
+				nhash_lookups++;
+#endif
+				return;
 			}
 			idx = (idx + 1) & mask;
 		}
 	}
 
-	return false;
+	/*
+	 * Use %p to print the reference, since most objects tracked by a resource owner
+	 * are pointers. It's a bit misleading if it's not a pointer, but this is a
+	 * programmer error, anyway.
+	 */
+	elog(ERROR, "%s %p is not owned by resource owner %s",
+		 kind->name, DatumGetPointer(value), ResourceOwnerGetName(owner));
 }
 
 /*
- * Get any convenient entry in a ResourceArray.
- *
- * "Convenient" is defined as "easy for ResourceArrayRemove to remove";
- * we help that along by setting lastidx to match.  This avoids O(N^2) cost
- * when removing all ResourceArray items during ResourceOwner destruction.
- *
- * Returns true if we found an element, or false if the array is empty.
+ * Call the ReleaseResource callback on entries with given 'phase'.
  */
-static bool
-ResourceArrayGetAny(ResourceArray *resarr, Datum *value)
+static void
+ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
+						bool printLeakWarnings)
 {
-	if (resarr->nitems == 0)
-		return false;
+	bool		found;
 
-	if (RESARRAY_IS_ARRAY(resarr))
-	{
-		/* Linear array: just return the first element. */
-		resarr->lastidx = 0;
-	}
-	else
+	/* First handle all the entries in the array. */
+	do
 	{
-		/* Hash: search forward from wherever we were last. */
-		uint32		mask = resarr->capacity - 1;
+		found = false;
 
-		for (;;)
+		for (int i = 0; i < owner->narr; i++)
 		{
-			resarr->lastidx &= mask;
-			if (resarr->itemsarr[resarr->lastidx] != resarr->invalidval)
-				break;
-			resarr->lastidx++;
+			if (owner->arr[i].kind->phase == phase)
+			{
+				Datum		value = owner->arr[i].item;
+				ResourceOwnerFuncs *kind = owner->arr[i].kind;
+
+				if (printLeakWarnings)
+					kind->PrintLeakWarning(value);
+				kind->ReleaseResource(value);
+				found = true;
+			}
 		}
-	}
 
-	*value = resarr->itemsarr[resarr->lastidx];
-	return true;
-}
+		/*
+		 * If any resources were released, check again because some of the
+		 * elements might have moved by the callbacks. We don't want to
+		 * miss them.
+		 */
+	} while (found && owner->narr > 0);
 
-/*
- * Trash a ResourceArray (we don't care about its state after this)
- */
-static void
-ResourceArrayFree(ResourceArray *resarr)
-{
-	if (resarr->itemsarr)
-		pfree(resarr->itemsarr);
+	/* Ok, the array has now been handled. Then the hash */
+	do
+	{
+		found = false;
+
+		for (int idx = 0; idx < owner->capacity; idx++)
+		{
+			if (owner->hash[idx].kind != NULL &&
+				owner->hash[idx].kind->phase == phase)
+			{
+				/* found one */
+				Datum		value = owner->hash[idx].item;
+				ResourceOwnerFuncs *kind = owner->hash[idx].kind;
+
+				if (printLeakWarnings)
+					kind->PrintLeakWarning(value);
+				kind->ReleaseResource(value);
+				found = true;
+			}
+		}
+		/*
+		 * Like with the array, we must check again after we reach the
+		 * end, if any callbacks were called. XXX: We could probably
+		 * stipulate that the callbacks may not do certain thing, like
+		 * remember more references in the same resource owner, to avoid
+		 * that.
+		 */
+	}
+	while (found && owner->nhash > 0);
 }

@@ -434,16 +437,10 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
 		parent->firstchild = owner;
 	}
 
-	ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
-	ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->planrefarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->tupdescarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->snapshotarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->filearr), FileGetDatum(-1));
-	ResourceArrayInit(&(owner->dsmarr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->jitarr), PointerGetDatum(NULL));
+#ifdef RESOWNER_TRACE
+	elog(LOG, "CREATE %d: %p %s",
+		 resowner_trace_counter++, owner, name);
+#endif
 
 	return owner;
 }
@@ -482,6 +479,15 @@ ResourceOwnerRelease(ResourceOwner owner,
 {
 	/* There's not currently any setup needed before recursing */
 	ResourceOwnerReleaseInternal(owner, phase, isCommit, isTopLevel);
+
+#ifdef RESOWNER_STATS
+	if (isTopLevel)
+	{
+		elog(LOG, "RESOWNER STATS: lookups: array %d, hash %d", narray_lookups, nhash_lookups);
+		narray_lookups = 0;
+		nhash_lookups = 0;
+	}
+#endif
 }
 
 static void
@@ -493,7 +499,6 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	ResourceOwner child;
 	ResourceOwner save;
 	ResourceReleaseCallbackItem *item;
-	Datum		foundres;
 
 	/* Recurse to handle descendants */
 	for (child = owner->firstchild; child != NULL; child = child->nextchild)
@@ -509,50 +514,13 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
 	{
 		/*
-		 * Release buffer pins.  Note that ReleaseBuffer will remove the
-		 * buffer entry from our array, so we just have to iterate till there
-		 * are none.
+		 * Release all references that need to be released before the locks.
 		 *
-		 * During a commit, there shouldn't be any remaining pins --- that
+		 * During a commit, there shouldn't be any remaining references --- that
 		 * would indicate failure to clean up the executor correctly --- so
 		 * issue warnings.  In the abort case, just clean up quietly.
 		 */
-		while (ResourceArrayGetAny(&(owner->bufferarr), &foundres))
-		{
-			Buffer		res = DatumGetBuffer(foundres);
-
-			if (isCommit)
-				PrintBufferLeakWarning(res);
-			ReleaseBuffer(res);
-		}
-
-		/* Ditto for relcache references */
-		while (ResourceArrayGetAny(&(owner->relrefarr), &foundres))
-		{
-			Relation	res = (Relation) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintRelCacheLeakWarning(res);
-			RelationClose(res);
-		}
-
-		/* Ditto for dynamic shared memory segments */
-		while (ResourceArrayGetAny(&(owner->dsmarr), &foundres))
-		{
-			dsm_segment *res = (dsm_segment *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintDSMLeakWarning(res);
-			dsm_detach(res);
-		}
-
-		/* Ditto for JIT contexts */
-		while (ResourceArrayGetAny(&(owner->jitarr), &foundres))
-		{
-			JitContext *context = (JitContext *) PointerGetDatum(foundres);
-
-			jit_release_context(context);
-		}
+		ResourceOwnerReleaseAll(owner, phase, isCommit);
 	}
 	else if (phase == RESOURCE_RELEASE_LOCKS)
 	{
@@ -561,7 +529,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 			/*
 			 * For a top-level xact we are going to release all locks (or at
 			 * least all non-session locks), so just do a single lmgr call at
-			 * the top of the recursion.
+			 * the top of the recursion
 			 */
 			if (owner == TopTransactionResourceOwner)
 			{
@@ -605,70 +573,9 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	else if (phase == RESOURCE_RELEASE_AFTER_LOCKS)
 	{
 		/*
-		 * Release catcache references.  Note that ReleaseCatCache will remove
-		 * the catref entry from our array, so we just have to iterate till
-		 * there are none.
-		 *
-		 * As with buffer pins, warn if any are left at commit time.
+		 * Release all references that need to be released after the locks.
 		 */
-		while (ResourceArrayGetAny(&(owner->catrefarr), &foundres))
-		{
-			HeapTuple	res = (HeapTuple) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintCatCacheLeakWarning(res);
-			ReleaseCatCache(res);
-		}
-
-		/* Ditto for catcache lists */
-		while (ResourceArrayGetAny(&(owner->catlistrefarr), &foundres))
-		{
-			CatCList   *res = (CatCList *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintCatCacheListLeakWarning(res);
-			ReleaseCatCacheList(res);
-		}
-
-		/* Ditto for plancache references */
-		while (ResourceArrayGetAny(&(owner->planrefarr), &foundres))
-		{
-			CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintPlanCacheLeakWarning(res);
-			ReleaseCachedPlan(res, true);
-		}
-
-		/* Ditto for tupdesc references */
-		while (ResourceArrayGetAny(&(owner->tupdescarr), &foundres))
-		{
-			TupleDesc	res = (TupleDesc) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintTupleDescLeakWarning(res);
-			DecrTupleDescRefCount(res);
-		}
-
-		/* Ditto for snapshot references */
-		while (ResourceArrayGetAny(&(owner->snapshotarr), &foundres))
-		{
-			Snapshot	res = (Snapshot) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintSnapshotLeakWarning(res);
-			UnregisterSnapshot(res);
-		}
-
-		/* Ditto for temporary files */
-		while (ResourceArrayGetAny(&(owner->filearr), &foundres))
-		{
-			File		res = DatumGetFile(foundres);
-
-			if (isCommit)
-				PrintFileLeakWarning(res);
-			FileClose(res);
-		}
+		ResourceOwnerReleaseAll(owner, phase, isCommit);
 	}
 
 	/* Let add-on modules get a chance too */
@@ -689,16 +596,42 @@ void
 ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner)
 {
 	ResourceOwner save;
-	Datum		foundres;
 
 	save = CurrentResourceOwner;
 	CurrentResourceOwner = owner;
-	while (ResourceArrayGetAny(&(owner->planrefarr), &foundres))
+
+	/* array first */
+	for (int i = 0; i < owner->narr; i++)
+	{
+		if (owner->arr[i].kind == &planref_resowner_funcs)
+		{
+			CachedPlan *planref = (CachedPlan *) DatumGetPointer(owner->arr[i].item);
+
+			owner->arr[i] = owner->arr[owner->narr - 1];
+			owner->narr--;
+			i--;
+
+			/* pass 'false' because we already removed the entry from the resowner */
+			ReleaseCachedPlan(planref, false);
+		}
+	}
+
+	/* Then hash */
+	for (int i = 0; i < owner->capacity; i++)
 	{
-		CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres);
+		if (owner->hash[i].kind == &planref_resowner_funcs)
+		{
+			CachedPlan *planref = (CachedPlan *) DatumGetPointer(owner->hash[i].item);
+
+			owner->hash[i].item = (Datum) 0;
+			owner->hash[i].kind = NULL;
+			owner->nhash--;
 
-		ReleaseCachedPlan(res, true);
+			/* pass 'false' because we already removed the entry from the resowner */
+			ReleaseCachedPlan(planref, false);
+		}
 	}
+
 	CurrentResourceOwner = save;
 }
 
@@ -715,18 +648,15 @@ ResourceOwnerDelete(ResourceOwner owner)
 	Assert(owner != CurrentResourceOwner);
 
 	/* And it better not own any resources, either */
-	Assert(owner->bufferarr.nitems == 0);
-	Assert(owner->catrefarr.nitems == 0);
-	Assert(owner->catlistrefarr.nitems == 0);
-	Assert(owner->relrefarr.nitems == 0);
-	Assert(owner->planrefarr.nitems == 0);
-	Assert(owner->tupdescarr.nitems == 0);
-	Assert(owner->snapshotarr.nitems == 0);
-	Assert(owner->filearr.nitems == 0);
-	Assert(owner->dsmarr.nitems == 0);
-	Assert(owner->jitarr.nitems == 0);
+	Assert(owner->narr == 0);
+	Assert(owner->nhash == 0);
 	Assert(owner->nlocks == 0 || owner->nlocks == MAX_RESOWNER_LOCKS + 1);
 
+#ifdef RESOWNER_TRACE
+	elog(LOG, "DELETE %d: %p %s",
+		 resowner_trace_counter++, owner, owner->name);
+#endif
+
 	/*
 	 * Delete children.  The recursive call will delink the child from me, so
 	 * just iterate as long as there is a child.
@@ -742,17 +672,8 @@ ResourceOwnerDelete(ResourceOwner owner)
 	ResourceOwnerNewParent(owner, NULL);
 
 	/* And free the object. */
-	ResourceArrayFree(&(owner->bufferarr));
-	ResourceArrayFree(&(owner->catrefarr));
-	ResourceArrayFree(&(owner->catlistrefarr));
-	ResourceArrayFree(&(owner->relrefarr));
-	ResourceArrayFree(&(owner->planrefarr));
-	ResourceArrayFree(&(owner->tupdescarr));
-	ResourceArrayFree(&(owner->snapshotarr));
-	ResourceArrayFree(&(owner->filearr));
-	ResourceArrayFree(&(owner->dsmarr));
-	ResourceArrayFree(&(owner->jitarr));
-
+	if (owner->hash)
+		pfree(owner->hash);
 	pfree(owner);
 }
 
@@ -807,6 +728,15 @@ ResourceOwnerNewParent(ResourceOwner owner,
 	}
 }
 
+/*
+ * Get the name of a resource owner, for debugging purposes
+ */
+const char *
+ResourceOwnerGetName(ResourceOwner owner)
+{
+	return owner->name;
+}
+
 /*
  * Register or deregister callback functions for resource cleanup
  *
@@ -905,44 +835,6 @@ ReleaseAuxProcessResourcesCallback(int code, Datum arg)
 	ReleaseAuxProcessResources(isCommit);
 }
 
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * buffer array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeBuffers(ResourceOwner owner)
-{
-	/* We used to allow pinning buffers without a resowner, but no more */
-	Assert(owner != NULL);
-	ResourceArrayEnlarge(&(owner->bufferarr));
-}
-
-/*
- * Remember that a buffer pin is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeBuffers()
- */
-void
-ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer)
-{
-	ResourceArrayAdd(&(owner->bufferarr), BufferGetDatum(buffer));
-}
-
-/*
- * Forget that a buffer pin is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
-{
-	if (!ResourceArrayRemove(&(owner->bufferarr), BufferGetDatum(buffer)))
-		elog(ERROR, "buffer %d is not owned by resource owner %s",
-			 buffer, owner->name);
-}
-
 /*
  * Remember that a Local Lock is owned by a ResourceOwner
  *
@@ -994,379 +886,3 @@ ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock)
 	elog(ERROR, "lock reference %p is not owned by resource owner %s",
 		 locallock, owner->name);
 }
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * catcache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeCatCacheRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->catrefarr));
-}
-
-/*
- * Remember that a catcache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeCatCacheRefs()
- */
-void
-ResourceOwnerRememberCatCacheRef(ResourceOwner owner, HeapTuple tuple)
-{
-	ResourceArrayAdd(&(owner->catrefarr), PointerGetDatum(tuple));
-}
-
-/*
- * Forget that a catcache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetCatCacheRef(ResourceOwner owner, HeapTuple tuple)
-{
-	if (!ResourceArrayRemove(&(owner->catrefarr), PointerGetDatum(tuple)))
-		elog(ERROR, "catcache reference %p is not owned by resource owner %s",
-			 tuple, owner->name);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * catcache-list reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeCatCacheListRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->catlistrefarr));
-}
-
-/*
- * Remember that a catcache-list reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeCatCacheListRefs()
- */
-void
-ResourceOwnerRememberCatCacheListRef(ResourceOwner owner, CatCList *list)
-{
-	ResourceArrayAdd(&(owner->catlistrefarr), PointerGetDatum(list));
-}
-
-/*
- * Forget that a catcache-list reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetCatCacheListRef(ResourceOwner owner, CatCList *list)
-{
-	if (!ResourceArrayRemove(&(owner->catlistrefarr), PointerGetDatum(list)))
-		elog(ERROR, "catcache list reference %p is not owned by resource owner %s",
-			 list, owner->name);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * relcache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeRelationRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->relrefarr));
-}
-
-/*
- * Remember that a relcache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeRelationRefs()
- */
-void
-ResourceOwnerRememberRelationRef(ResourceOwner owner, Relation rel)
-{
-	ResourceArrayAdd(&(owner->relrefarr), PointerGetDatum(rel));
-}
-
-/*
- * Forget that a relcache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetRelationRef(ResourceOwner owner, Relation rel)
-{
-	if (!ResourceArrayRemove(&(owner->relrefarr), PointerGetDatum(rel)))
-		elog(ERROR, "relcache reference %s is not owned by resource owner %s",
-			 RelationGetRelationName(rel), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintRelCacheLeakWarning(Relation rel)
-{
-	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
-		 RelationGetRelationName(rel));
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * plancache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargePlanCacheRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->planrefarr));
-}
-
-/*
- * Remember that a plancache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargePlanCacheRefs()
- */
-void
-ResourceOwnerRememberPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
-{
-	ResourceArrayAdd(&(owner->planrefarr), PointerGetDatum(plan));
-}
-
-/*
- * Forget that a plancache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
-{
-	if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)))
-		elog(ERROR, "plancache reference %p is not owned by resource owner %s",
-			 plan, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintPlanCacheLeakWarning(CachedPlan *plan)
-{
-	elog(WARNING, "plancache reference leak: plan %p not closed", plan);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * tupdesc reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeTupleDescs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->tupdescarr));
-}
-
-/*
- * Remember that a tupdesc reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeTupleDescs()
- */
-void
-ResourceOwnerRememberTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
-{
-	ResourceArrayAdd(&(owner->tupdescarr), PointerGetDatum(tupdesc));
-}
-
-/*
- * Forget that a tupdesc reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
-{
-	if (!ResourceArrayRemove(&(owner->tupdescarr), PointerGetDatum(tupdesc)))
-		elog(ERROR, "tupdesc reference %p is not owned by resource owner %s",
-			 tupdesc, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintTupleDescLeakWarning(TupleDesc tupdesc)
-{
-	elog(WARNING,
-		 "TupleDesc reference leak: TupleDesc %p (%u,%d) still referenced",
-		 tupdesc, tupdesc->tdtypeid, tupdesc->tdtypmod);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * snapshot reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeSnapshots(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->snapshotarr));
-}
-
-/*
- * Remember that a snapshot reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeSnapshots()
- */
-void
-ResourceOwnerRememberSnapshot(ResourceOwner owner, Snapshot snapshot)
-{
-	ResourceArrayAdd(&(owner->snapshotarr), PointerGetDatum(snapshot));
-}
-
-/*
- * Forget that a snapshot reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetSnapshot(ResourceOwner owner, Snapshot snapshot)
-{
-	if (!ResourceArrayRemove(&(owner->snapshotarr), PointerGetDatum(snapshot)))
-		elog(ERROR, "snapshot reference %p is not owned by resource owner %s",
-			 snapshot, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintSnapshotLeakWarning(Snapshot snapshot)
-{
-	elog(WARNING, "Snapshot reference leak: Snapshot %p still referenced",
-		 snapshot);
-}
-
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * files reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeFiles(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->filearr));
-}
-
-/*
- * Remember that a temporary file is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeFiles()
- */
-void
-ResourceOwnerRememberFile(ResourceOwner owner, File file)
-{
-	ResourceArrayAdd(&(owner->filearr), FileGetDatum(file));
-}
-
-/*
- * Forget that a temporary file is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetFile(ResourceOwner owner, File file)
-{
-	if (!ResourceArrayRemove(&(owner->filearr), FileGetDatum(file)))
-		elog(ERROR, "temporary file %d is not owned by resource owner %s",
-			 file, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintFileLeakWarning(File file)
-{
-	elog(WARNING, "temporary file leak: File %d still referenced",
-		 file);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * dynamic shmem segment reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeDSMs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->dsmarr));
-}
-
-/*
- * Remember that a dynamic shmem segment is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeDSMs()
- */
-void
-ResourceOwnerRememberDSM(ResourceOwner owner, dsm_segment *seg)
-{
-	ResourceArrayAdd(&(owner->dsmarr), PointerGetDatum(seg));
-}
-
-/*
- * Forget that a dynamic shmem segment is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetDSM(ResourceOwner owner, dsm_segment *seg)
-{
-	if (!ResourceArrayRemove(&(owner->dsmarr), PointerGetDatum(seg)))
-		elog(ERROR, "dynamic shared memory segment %u is not owned by resource owner %s",
-			 dsm_segment_handle(seg), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintDSMLeakWarning(dsm_segment *seg)
-{
-	elog(WARNING, "dynamic shared memory leak: segment %u still referenced",
-		 dsm_segment_handle(seg));
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * JIT context reference array.
- *
- * This is separate from actually inserting an entry because if we run out of
- * memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeJIT(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->jitarr));
-}
-
-/*
- * Remember that a JIT context is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeJIT()
- */
-void
-ResourceOwnerRememberJIT(ResourceOwner owner, Datum handle)
-{
-	ResourceArrayAdd(&(owner->jitarr), handle);
-}
-
-/*
- * Forget that a JIT context is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle)
-{
-	if (!ResourceArrayRemove(&(owner->jitarr), handle))
-		elog(ERROR, "JIT context %p is not owned by resource owner %s",
-			 DatumGetPointer(handle), owner->name);
-}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 8c41483e87c..b3ea3ce7650 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -66,7 +66,7 @@
 #include "utils/memutils.h"
 #include "utils/old_snapshot.h"
 #include "utils/rel.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/timestamp.h"
@@ -174,6 +174,18 @@ static Snapshot CopySnapshot(Snapshot snapshot);
 static void FreeSnapshot(Snapshot snapshot);
 static void SnapshotResetXmin(void);
 
+/* ResourceOwner callbacks to track snapshot references */
+static void ResOwnerReleaseSnapshot(Datum res);
+static void ResOwnerPrintSnapshotLeakWarning(Datum res);
+
+static ResourceOwnerFuncs snapshot_resowner_funcs =
+{
+	.name = "snapshot reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseSnapshot,
+	.PrintLeakWarning = ResOwnerPrintSnapshotLeakWarning
+};
+
 /*
  * Snapshot fields to be serialized.
  *
@@ -831,9 +843,10 @@ RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
 	snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
 
 	/* and tell resowner.c about it */
-	ResourceOwnerEnlargeSnapshots(owner);
+	ResourceOwnerEnlarge(owner);
 	snap->regd_count++;
-	ResourceOwnerRememberSnapshot(owner, snap);
+	ResourceOwnerRemember(owner, PointerGetDatum(snap),
+						  &snapshot_resowner_funcs);
 
 	if (snap->regd_count == 1)
 		pairingheap_add(&RegisteredSnapshots, &snap->ph_node);
@@ -870,7 +883,8 @@ UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner)
 	Assert(snapshot->regd_count > 0);
 	Assert(!pairingheap_is_empty(&RegisteredSnapshots));
 
-	ResourceOwnerForgetSnapshot(owner, snapshot);
+	ResourceOwnerForget(owner, PointerGetDatum(snapshot),
+						&snapshot_resowner_funcs);
 
 	snapshot->regd_count--;
 	if (snapshot->regd_count == 0)
@@ -2345,3 +2359,19 @@ XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot)
 
 	return false;
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseSnapshot(Datum res)
+{
+	UnregisterSnapshot((Snapshot) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintSnapshotLeakWarning(Datum res)
+{
+	elog(WARNING, "Snapshot reference leak: Snapshot %p still referenced",
+		 DatumGetPointer(res));
+}
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 3377fa56768..c4aa7d7e31e 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -296,6 +296,21 @@ typedef struct CkptSortItem
 
 extern CkptSortItem *CkptBufferIds;
 
+/* ResourceOwner callbacks to hold buffer pins */
+extern ResourceOwnerFuncs buffer_resowner_funcs;
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+static inline void
+ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer)
+{
+	ResourceOwnerRemember(owner, Int32GetDatum(buffer), &buffer_resowner_funcs);
+}
+static inline void
+ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
+{
+	ResourceOwnerForget(owner, Int32GetDatum(buffer), &buffer_resowner_funcs);
+}
+
 /*
  * Internal buffer management routines
  */
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index f4aa316604e..dbb10dfdd16 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -225,7 +225,4 @@ extern void PrepareToInvalidateCacheTuple(Relation relation,
 										  HeapTuple newtuple,
 										  void (*function) (int, uint32, Oid));
 
-extern void PrintCatCacheLeakWarning(HeapTuple tuple);
-extern void PrintCatCacheListLeakWarning(CatCList *list);
-
 #endif							/* CATCACHE_H */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 4901568553c..2e0d5006719 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -233,4 +233,6 @@ extern bool CachedPlanIsSimplyValid(CachedPlanSource *plansource,
 extern CachedExpression *GetCachedExpression(Node *expr);
 extern void FreeCachedExpression(CachedExpression *cexpr);
 
+extern ResourceOwnerFuncs planref_resowner_funcs;
+
 #endif							/* PLANCACHE_H */
diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h
index 878f39ccf14..87a2bf0558e 100644
--- a/src/include/utils/resowner.h
+++ b/src/include/utils/resowner.h
@@ -50,6 +50,32 @@ typedef enum
 	RESOURCE_RELEASE_AFTER_LOCKS
 } ResourceReleasePhase;
 
+/*
+ * In order to track an object, resowner.c needs a few callbacks for it.
+ * The callbacks for an object of a specific kind are encapsulated in
+ * ResourceOwnerFuncs.
+ */
+typedef struct ResourceOwnerFuncs
+{
+	const char *name;		/* name for the object kind, for debugging */
+	ResourceReleasePhase phase; /* when are these objects released? */
+
+	/*
+	 * Release resource.
+	 *
+	 * NOTE: this must call ResourceOwnerForget to disassociate it with the
+	 * resource owner.
+	 */
+	void (*ReleaseResource)(Datum res);
+
+	/*
+	 * Print a warning, when a resource has not been properly released before
+	 * commit.
+	 */
+	void (*PrintLeakWarning)(Datum res);
+
+} ResourceOwnerFuncs;
+
 /*
  *	Dynamically loaded modules can get control during ResourceOwnerRelease
  *	by providing a callback of this form.
@@ -71,16 +97,30 @@ extern void ResourceOwnerRelease(ResourceOwner owner,
 								 ResourceReleasePhase phase,
 								 bool isCommit,
 								 bool isTopLevel);
-extern void ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner);
 extern void ResourceOwnerDelete(ResourceOwner owner);
 extern ResourceOwner ResourceOwnerGetParent(ResourceOwner owner);
 extern void ResourceOwnerNewParent(ResourceOwner owner,
 								   ResourceOwner newparent);
+extern const char *ResourceOwnerGetName(ResourceOwner owner);
+
+extern void ResourceOwnerEnlarge(ResourceOwner owner);
+extern void ResourceOwnerRemember(ResourceOwner owner, Datum res, ResourceOwnerFuncs *kind);
+extern void ResourceOwnerForget(ResourceOwner owner, Datum res, ResourceOwnerFuncs *kind);
+
 extern void RegisterResourceReleaseCallback(ResourceReleaseCallback callback,
 											void *arg);
 extern void UnregisterResourceReleaseCallback(ResourceReleaseCallback callback,
 											  void *arg);
+
 extern void CreateAuxProcessResourceOwner(void);
 extern void ReleaseAuxProcessResources(bool isCommit);
 
+/* special support for local lock management */
+struct LOCALLOCK;
+extern void ResourceOwnerRememberLock(ResourceOwner owner, struct LOCALLOCK *locallock);
+extern void ResourceOwnerForgetLock(ResourceOwner owner, struct LOCALLOCK *locallock);
+
+/* special function to relase all plancache references */
+extern void ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner);
+
 #endif							/* RESOWNER_H */
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
deleted file mode 100644
index a781a7a2aa6..00000000000
--- a/src/include/utils/resowner_private.h
+++ /dev/null
@@ -1,98 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * resowner_private.h
- *	  POSTGRES resource owner private definitions.
- *
- * See utils/resowner/README for more info.
- *
- *
- * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/include/utils/resowner_private.h
- *
- *-------------------------------------------------------------------------
- */
-#ifndef RESOWNER_PRIVATE_H
-#define RESOWNER_PRIVATE_H
-
-#include "storage/dsm.h"
-#include "storage/fd.h"
-#include "storage/lock.h"
-#include "utils/catcache.h"
-#include "utils/plancache.h"
-#include "utils/resowner.h"
-#include "utils/snapshot.h"
-
-
-/* support for buffer refcount management */
-extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
-extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
-extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
-
-/* support for local lock management */
-extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
-extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
-
-/* support for catcache refcount management */
-extern void ResourceOwnerEnlargeCatCacheRefs(ResourceOwner owner);
-extern void ResourceOwnerRememberCatCacheRef(ResourceOwner owner,
-											 HeapTuple tuple);
-extern void ResourceOwnerForgetCatCacheRef(ResourceOwner owner,
-										   HeapTuple tuple);
-extern void ResourceOwnerEnlargeCatCacheListRefs(ResourceOwner owner);
-extern void ResourceOwnerRememberCatCacheListRef(ResourceOwner owner,
-												 CatCList *list);
-extern void ResourceOwnerForgetCatCacheListRef(ResourceOwner owner,
-											   CatCList *list);
-
-/* support for relcache refcount management */
-extern void ResourceOwnerEnlargeRelationRefs(ResourceOwner owner);
-extern void ResourceOwnerRememberRelationRef(ResourceOwner owner,
-											 Relation rel);
-extern void ResourceOwnerForgetRelationRef(ResourceOwner owner,
-										   Relation rel);
-
-/* support for plancache refcount management */
-extern void ResourceOwnerEnlargePlanCacheRefs(ResourceOwner owner);
-extern void ResourceOwnerRememberPlanCacheRef(ResourceOwner owner,
-											  CachedPlan *plan);
-extern void ResourceOwnerForgetPlanCacheRef(ResourceOwner owner,
-											CachedPlan *plan);
-
-/* support for tupledesc refcount management */
-extern void ResourceOwnerEnlargeTupleDescs(ResourceOwner owner);
-extern void ResourceOwnerRememberTupleDesc(ResourceOwner owner,
-										   TupleDesc tupdesc);
-extern void ResourceOwnerForgetTupleDesc(ResourceOwner owner,
-										 TupleDesc tupdesc);
-
-/* support for snapshot refcount management */
-extern void ResourceOwnerEnlargeSnapshots(ResourceOwner owner);
-extern void ResourceOwnerRememberSnapshot(ResourceOwner owner,
-										  Snapshot snapshot);
-extern void ResourceOwnerForgetSnapshot(ResourceOwner owner,
-										Snapshot snapshot);
-
-/* support for temporary file management */
-extern void ResourceOwnerEnlargeFiles(ResourceOwner owner);
-extern void ResourceOwnerRememberFile(ResourceOwner owner,
-									  File file);
-extern void ResourceOwnerForgetFile(ResourceOwner owner,
-									File file);
-
-/* support for dynamic shared memory management */
-extern void ResourceOwnerEnlargeDSMs(ResourceOwner owner);
-extern void ResourceOwnerRememberDSM(ResourceOwner owner,
-									 dsm_segment *);
-extern void ResourceOwnerForgetDSM(ResourceOwner owner,
-								   dsm_segment *);
-
-/* support for JITContext management */
-extern void ResourceOwnerEnlargeJIT(ResourceOwner owner);
-extern void ResourceOwnerRememberJIT(ResourceOwner owner,
-									 Datum handle);
-extern void ResourceOwnerForgetJIT(ResourceOwner owner,
-								   Datum handle);
-
-#endif							/* RESOWNER_PRIVATE_H */

---

From: Michael Paquier <[email-masked]>
Date: Wed, 18 Nov 2020 17:06:48 +0900

On Tue, Nov 17, 2020 at 04:21:29PM +0200, Heikki Linnakangas wrote:
> 2. It's difficult for extensions to use. There is a callback mechanism for
> extensions, but it's much less convenient to use than the built-in
> patch [2] would move that support for tracking OpenSSL contexts to the core,
> which makes it a lot more convenient for pgcrypto. Wouldn't it be nice if
> extensions could have the same ergonomics as built-in code, if they need to
> track external resources?

+1.  True that the current interface is a bit hard to grasp, one can
easily miss that showing reference leaks is very important if an
allocation happens out of the in-core palloc machinery at commit time.
(The patch you are referring to is not really popular, but that does
not prevent this thread to move on on its own.)

+ * Size of the small fixed-size array to hold most-recently remembered resources.
  */
-#define RESARRAY_INIT_SIZE 16
+#define RESOWNER_ARRAY_SIZE 8
Why did you choose this size for the initial array?

+extern const char *ResourceOwnerGetName(ResourceOwner owner);
This is only used in resowner.c, at one place.

-   ResourceOwnerForgetJIT(context->resowner, PointerGetDatum(context));
[...]
+   ResourceOwnerForget(context->resowner, PointerGetDatum(context), &jit_funcs);
This moves the JIT context release from jit.c to llvm.c.  I think
that's indeed more consistent, and correct.  It would be better to
check this one with Andres, though.

> testing with Kyotaro's "catcachebench" to stress-test syscache lookups, this
> performs a little better. In that test, all the activity happens in the
> small array portion, but I believe that's true for most use cases.

> Thoughts? Can anyone suggest test scenarios to verify the performance of

Playing with catcache.c is the first thing that came to my mind.
After that some micro-benchmarking with DSM or snapshots?  I am not
sure if we would notice a difference in a real-life scenario, say even
a pgbench -S with prepared queries.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 18 Nov 2020 10:50:08 +0200

> Why did you choose this size for the initial array?

Just a guess. The old init size was 16 Datums. The entries in the new 
array are twice as large, Datum+pointer.

The "RESOWNER_STATS" #ifdef blocks can be enabled to check how many 
lookups fit in the array. With pgbench, RESOWNER_ARRAY_SIZE 8:

RESOWNER STATS: lookups: array 235, hash 32

If RESOWNER_ARRAY_STATS is increased to 16, all the lookups fit in the 
array. But I haven't done any benchmarking to see which is faster.

BTW, I think there would be an easy win in the hashing codepath, by 
changing to a cheaper hash function. Currently, with or without this 
patch, we use hash_any(). Changing that to murmurhash32() or something 
similar would be a drop-in replacement.

- Heikki

---

From: Michael Paquier <[email-masked]>
Date: Thu, 19 Nov 2020 11:16:05 +0900

> array. But I haven't done any benchmarking to see which is faster.

My gut tells me that your guess is right, but it would be better to be
sure.

> BTW, I think there would be an easy win in the hashing codepath, by changing
> to a cheaper hash function. Currently, with or without this patch, we use
> hash_any(). Changing that to murmurhash32() or something similar would be a
> drop-in replacement.

Good idea.
--
Michael

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 19 Nov 2020 17:27:18 +0800

> > hash_any(). Changing that to murmurhash32() or something similar would be a

+1, and +1 for this refactoring.

I just saw a minor issue in a comment while reviewing the patch:

[...]

The existing comment is kept as-is, but it should mention that it's
now the hash capacity that is increased.

---

From: Craig Ringer <[email-masked]>
Date: Thu, 19 Nov 2020 18:40:19 +0800

[off-list for now]

> they need to track external resources?

Yes, in general I'm a big fan of making life easier for extensions (see
postscript), and this looks helpful. It's certainly fairly clear to read.
I'm in-principle in favour, though I haven't read the old resowner code
closely enough to have a strong opinion.

> Thoughts? Can anyone suggest test scenarios to verify the performance of

I didn't think of much really.

I have tossed together a patch on top of yours that adds some
systemtap/dtrace tracepoints and provides a demo systemtap script that
shows some basic stats collection done using them. My intention was to make
it easier to see where the work in the resource owner code was being done.
But after I did the initial version I realised that in this case it
probably doesn't add a lot of value over using targeted profiling of only
functions in resowner.c and their callees which is easy enough using perf
and regular DWARF debuginfo. So I didn't land up polishing it and adding
stats for the other counters. It's attached anyway, in case it's
interesting or useful to anyone.

P.S. At some point I want to tackle some things similar to what you've done
here for other kinds of backend resources. In particular, to allow
extensions to register their own heavyweight lock types - with the ability
to handle lock timeouts, and add callbacks in the deadlock detector to
allow extensions to register dependency edges that are not natively visible
to the deadlock detector and to choose victim priorities. Also some
enhancements to how pg_depend tracking can be used by extensions. And I
want to help get the proposed custom ProcSignal changes in too. I think
there's a whole lot to be done to make extensions easier and safer to
author in general, like providing a simple way to do error trapping and
recovery in an extension mainloop without copy/pasting a bunch of
PostgresMain code, better default signal handlers, startup/shutdown that
shares more with user backends, etc. Right now it's quite tricky to get
bgworkers to behave well. </wildhandwaving>
From 9f3e6978b1bbfeb8ca3f1cac42e86c4731143404 Mon Sep 17 00:00:00 2001
From: Craig Ringer <[email-masked]>
Date: Wed, 18 Nov 2020 14:30:08 +0800
Subject: [PATCH] Poc of systemtap probes for resowner timings

---
 resowner.stp                          | 106 ++++++++++++++++++++++++++
 src/backend/utils/probes.d            |  22 ++++++
 src/backend/utils/resowner/resowner.c |  96 +++++++++++++++++++++--
 3 files changed, 216 insertions(+), 8 deletions(-)
 create mode 100644 resowner.stp

diff --git a/resowner.stp b/resowner.stp
new file mode 100644
index 0000000000..24b37b39e6
--- /dev/null
+++ b/resowner.stp
@@ -0,0 +1,106 @@
+# PATH="$(dirname $(realpath $(find build/ -name postgres -type f))):$PATH" /usr/local/systemtap/bin/stap -v ./resowner.stp
+
+/*
+.mark("resowner__created") $arg1:long $arg2:long
+.mark("resowner__delete__done") $arg1:long $arg2:long
+.mark("resowner__delete__start") $arg1:long $arg2:long
+.mark("resowner__enlarge__done") $arg1:long $arg2:long $arg3:long
+.mark("resowner__enlarge__skipped") $arg1:long $arg2:long
+.mark("resowner__enlarge__start") $arg1:long $arg2:long $arg3:long $arg4:long
+.mark("resowner__forget__done") $arg1:long $arg2:long $arg3:long $arg4:long $arg5:long
+.mark("resowner__forget__start") $arg1:long $arg2:long $arg3:long $arg4:long
+.mark("resowner__new__parent__done") $arg1:long
+.mark("resowner__new__parent__start") $arg1:long $arg2:long $arg3:long
+.mark("resowner__release__all__done") $arg1:long $arg2:long $arg3:long
+.mark("resowner__release__all__hash")
+.mark("resowner__release__all__start") $arg1:long $arg2:long $arg3:long $arg4:long $arg5:long
+.mark("resowner__release__done") $arg1:long $arg2:long
+.mark("resowner__release__plancacherefs__done") $arg1:long $arg2:long
+.mark("resowner__release__plancacherefs__start") $arg1:long $arg2:long
+.mark("resowner__release__start") $arg1:long $arg2:long $arg3:long $arg4:long $arg5:long
+.mark("resowner__remembered") $arg1:long $arg2:long $arg3:long $arg4:long
+*/
+
+@define probe_prefix %( sprintf("[%6d] %20s %20s %8x %20s", pid(), application_names[pid()], $$name, owner, owner_name) %)
+
+probe pg = process("postgres") {}
+
+# stats
+global operation_stats;
+
+# backend info
+private application_names;
+
+# in-progress operations
+private track_resowner_deletes;
+
+probe pg.begin {
+	if (@defined(@var("[email-masked]","postgres"))) {
+		application_names[pid()] = user_string(@var("[email-masked]","postgres"),"<?>")
+	} else {
+		application_names[pid()] = "<?>"
+	}
+	printf("[%6d] started %s\n", pid(), application_names[pid()])
+}
+
+probe pg.end {
+	delete application_names[pid()];
+	delete track_resowner_deletes[pid(),*];
+}
+
+probe pg.mark("resowner__created") {
+	owner = $arg1
+	owner_name = user_string($arg2)
+	printf("%s\n", @probe_prefix)
+}
+
+probe pg.mark("resowner__delete__start") {
+	owner = $arg1
+	owner_name = user_string($arg2)
+	track_resowner_deletes[pid(),owner] = gettimeofday_us()
+}
+
+probe pg.mark("resowner__delete__done") {
+	owner = $arg1
+	owner_name = user_string($arg2)
+	starttime_us = track_resowner_deletes[pid(),owner]
+	if (starttime_us != 0) {
+		elapsed = gettimeofday_us() - starttime_us
+		operation_stats["resowner_delete"] <<< elapsed
+		printf("%s: %s\n", @probe_prefix, usecs_to_string(elapsed));
+	}
+	delete track_resowner_deletes[pid(),owner]
+}
+
+function print_stat(statname) {
+	count = @count(operation_stats[statname])
+	if (count > 0) {
+		min = @min(operation_stats[statname])
+		max = @max(operation_stats[statname])
+		printf("-- %s:\n"
+		       "--    count:  %6d\n"
+		       "--    mean:   %6d\n"
+		       "--    stddev: %6d\n"
+		       "--    min:    %6d\n"
+		       "--    max:    %6d\n",
+		       statname,
+		       count,
+		       @avg(operation_stats[statname]),
+		       @variance(operation_stats[statname])/count,
+		       min, max
+		)
+		println(@hist_log(operation_stats[statname]))
+	}
+}
+
+function print_stats() {
+	print_stat("resowner_delete")
+}
+
+probe timer.ms(5000) {
+	print_stats()
+}
+
+probe end {
+	print_stats()
+ }
diff --git a/src/backend/utils/probes.d b/src/backend/utils/probes.d
index a0b0458108..7be815f9de 100644
--- a/src/backend/utils/probes.d
+++ b/src/backend/utils/probes.d
@@ -21,6 +21,9 @@
 #define Oid unsigned int
 #define ForkNumber int
 #define bool unsigned char
+#define Datum long int
+#define ResourceOwnerData void
+#define ResourceReleasePhase int
 
 provider postgresql {
 
@@ -91,4 +94,23 @@ provider postgresql {
 	probe wal__switch();
 	probe wal__buffer__write__dirty__start();
 	probe wal__buffer__write__dirty__done();
+
+	probe resowner__created(ResourceOwnerData*, const char *);
+	probe resowner__remembered(ResourceOwnerData*, const char *, Datum, const char *);
+	probe resowner__enlarge__skipped(ResourceOwnerData*, const char*);
+	probe resowner__enlarge__start(ResourceOwnerData*, const char *, int, int);
+	probe resowner__enlarge__done(ResourceOwnerData*, const char *, int);
+	probe resowner__forget__start(ResourceOwnerData*, const char *, Datum, const char *);
+	probe resowner__forget__done(ResourceOwnerData*, const char *, Datum, int, int);
+	probe resowner__release__all__start(ResourceOwnerData*, const char *, ResourceReleasePhase, int, int);
+	probe resowner__release__all__hash();
+	probe resowner__release__all__done(ResourceOwnerData*, const char *, ResourceReleasePhase);
+	probe resowner__release__start(ResourceOwnerData*, const char *, int, bool, bool);
+	probe resowner__release__done(ResourceOwnerData*, const char *);
+	probe resowner__delete__start(ResourceOwnerData*, const char *);
+	probe resowner__delete__done(long int, const char *);
+	probe resowner__release__plancacherefs__start(ResourceOwnerData*, const char *);
+	probe resowner__release__plancacherefs__done(ResourceOwnerData*, const char *);
+	probe resowner__new__parent__start(ResourceOwnerData*, ResourceOwnerData*, ResourceOwnerData*);
+	probe resowner__new__parent__done(ResourceOwnerData*);
 };
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 2e1ae4f8e0..fcd2c6f61d 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -26,6 +26,7 @@
 #include "storage/proc.h"
 #include "utils/memutils.h"
 #include "utils/plancache.h"
+#include "utils/probes.h"
 #include "utils/resowner.h"
 
 /*
@@ -152,10 +153,18 @@ static ResourceReleaseCallbackItem *ResourceRelease_callbacks = NULL;

 /* Internal routines */
-static void ResourceOwnerReleaseInternal(ResourceOwner owner,
+static void ResourceOwnerReleaseRecurse(ResourceOwner owner,
 										 ResourceReleasePhase phase,
 										 bool isCommit,
 										 bool isTopLevel);
+static void ResourceOwnerReleaseSelf(ResourceOwner owner,
+										 ResourceReleasePhase phase,
+										 bool isCommit,
+										 bool isTopLevel);
+static void ResourceOwnerReleaseCallbacks(ResourceOwner owner,
+                               ResourceReleasePhase phase,
+                               bool isCommit,
+                               bool isTopLevel);
 static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);

@@ -194,9 +203,13 @@ void
 ResourceOwnerEnlarge(ResourceOwner owner)
 {
 	if (owner->narr < RESOWNER_ARRAY_SIZE)
+	{
+		TRACE_POSTGRESQL_RESOWNER_ENLARGE_SKIPPED(owner, owner->name);
 		return;					/* no work needed */
+	}
 
 	/* Is there space in the hash? If not, enlarge it. */
+	TRACE_POSTGRESQL_RESOWNER_ENLARGE_START(owner, owner->name, owner->nhash, owner->narr);
 
 	/* Double the capacity of the array (capacity must stay a power of 2!) */
 	if (owner->narr + owner->nhash >= owner->grow_at)
@@ -246,6 +259,8 @@ ResourceOwnerEnlarge(ResourceOwner owner)
 	owner->narr = 0;
 
 	Assert(owner->nhash < owner->grow_at);
+
+	TRACE_POSTGRESQL_RESOWNER_ENLARGE_DONE(owner, owner->name, owner->nhash);
 }
 
 /*
@@ -270,6 +285,8 @@ ResourceOwnerRemember(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind
 	owner->arr[idx].item = value;
 	owner->arr[idx].kind = kind;
 	owner->narr++;
+
+	TRACE_POSTGRESQL_RESOWNER_REMEMBERED(owner, owner->name, value, kind->name);
 }
 
 /*
@@ -292,6 +309,8 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 		 resowner_trace_counter++, owner, DatumGetUInt64(value), kind->name);
 #endif
 
+	TRACE_POSTGRESQL_RESOWNER_FORGET_START(owner, owner->name, value, kind->name);
+
 	/* Search through all items, but check the array first. */
 	for (i = 0; i < owner->narr; i++)
 	{
@@ -305,6 +324,7 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 			narray_lookups++;
 #endif
 
+			TRACE_POSTGRESQL_RESOWNER_FORGET_DONE(owner, owner->name, value, i, 0);
 			return;
 		}
 	}
@@ -327,6 +347,7 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
 #ifdef RESOWNER_STATS
 				nhash_lookups++;
 #endif
+				TRACE_POSTGRESQL_RESOWNER_FORGET_DONE(owner, owner->name, value, owner->narr, i);
 				return;
 			}
 			idx = (idx + 1) & mask;
@@ -351,6 +372,9 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
 {
 	bool		found;
 
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_ALL_START(owner, owner->name, phase,
+					owner->narr, owner->nhash);
+
 	/* First handle all the entries in the array. */
 	do
 	{
@@ -377,6 +401,9 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
 		 */
 	} while (found && owner->narr > 0);
 
+	/* For probe/trace tools to time array and hash release separately */
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_ALL_HASH();
+
 	/* Ok, the array has now been handled. Then the hash */
 	do
 	{
@@ -406,6 +433,8 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
 		 */
 	}
 	while (found && owner->nhash > 0);
+
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_ALL_DONE(owner, owner->name, phase);
 }

@@ -420,6 +449,9 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
  *
  * All ResourceOwner objects are kept in TopMemoryContext, since they should
  * only be freed explicitly.
+ *
+ * The owner name is not copied. It should generally be a string constant. Otherwise
+ * the 
  */
 ResourceOwner
 ResourceOwnerCreate(ResourceOwner parent, const char *name)
@@ -442,6 +474,8 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
 		 resowner_trace_counter++, owner, name);
 #endif
 
+	TRACE_POSTGRESQL_RESOWNER_CREATED(owner, owner->name);
+
 	return owner;
 }
 
@@ -478,7 +512,7 @@ ResourceOwnerRelease(ResourceOwner owner,
 					 bool isTopLevel)
 {
 	/* There's not currently any setup needed before recursing */
-	ResourceOwnerReleaseInternal(owner, phase, isCommit, isTopLevel);
+	ResourceOwnerReleaseRecurse(owner, phase, isCommit, isTopLevel);
 
 #ifdef RESOWNER_STATS
 	if (isTopLevel)
@@ -491,18 +525,19 @@ ResourceOwnerRelease(ResourceOwner owner,
 }
 
 static void
-ResourceOwnerReleaseInternal(ResourceOwner owner,
+ResourceOwnerReleaseRecurse(ResourceOwner owner,
 							 ResourceReleasePhase phase,
 							 bool isCommit,
 							 bool isTopLevel)
 {
 	ResourceOwner child;
 	ResourceOwner save;
-	ResourceReleaseCallbackItem *item;
+
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_START(owner, owner->name, phase, isCommit, isTopLevel);
 
 	/* Recurse to handle descendants */
 	for (child = owner->firstchild; child != NULL; child = child->nextchild)
-		ResourceOwnerReleaseInternal(child, phase, isCommit, isTopLevel);
+		ResourceOwnerReleaseRecurse(child, phase, isCommit, isTopLevel);
 
 	/*
 	 * Make CurrentResourceOwner point to me, so that ReleaseBuffer etc don't
@@ -511,6 +546,27 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	save = CurrentResourceOwner;
 	CurrentResourceOwner = owner;
 
+	/* Release directly managed resources */
+	ResourceOwnerReleaseSelf(owner, phase, isCommit, isTopLevel);
+
+	/* And run any release callbacks */
+	ResourceOwnerReleaseCallbacks(owner, phase, isCommit, isTopLevel);
+
+	CurrentResourceOwner = save;
+
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_DONE(owner, owner->name);
+}
+
+/*
+ * Non-recursive part of ResourceOwnerRelease for a single resowner's
+ * resources.
+ */
+static void
+ResourceOwnerReleaseSelf(ResourceOwner owner,
+                               ResourceReleasePhase phase,
+                               bool isCommit,
+                               bool isTopLevel)
+{
 	if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
 	{
 		/*
@@ -577,12 +633,18 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 		 */
 		ResourceOwnerReleaseAll(owner, phase, isCommit);
 	}
+}
+
+static void
+ResourceOwnerReleaseCallbacks(ResourceOwner owner,
+                               ResourceReleasePhase phase,
+                               bool isCommit,
+                               bool isTopLevel)
+{
+	ResourceReleaseCallbackItem *item;
 
-	/* Let add-on modules get a chance too */
 	for (item = ResourceRelease_callbacks; item; item = item->next)
 		item->callback(phase, isCommit, isTopLevel, item->arg);
-
-	CurrentResourceOwner = save;
 }
 
 /*
@@ -600,6 +662,8 @@ ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner)
 	save = CurrentResourceOwner;
 	CurrentResourceOwner = owner;
 
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_PLANCACHEREFS_START(owner, owner->name);
+
 	/* array first */
 	for (int i = 0; i < owner->narr; i++)
 	{
@@ -633,6 +697,8 @@ ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner)
 	}
 
 	CurrentResourceOwner = save;
+
+	TRACE_POSTGRESQL_RESOWNER_RELEASE_PLANCACHEREFS_DONE(owner, owner->name);
 }
 
 /*
@@ -644,6 +710,8 @@ ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner)
 void
 ResourceOwnerDelete(ResourceOwner owner)
 {
+	const char * const name = owner->name;
+
 	/* We had better not be deleting CurrentResourceOwner ... */
 	Assert(owner != CurrentResourceOwner);
 
@@ -657,6 +725,8 @@ ResourceOwnerDelete(ResourceOwner owner)
 		 resowner_trace_counter++, owner, owner->name);
 #endif
 
+	TRACE_POSTGRESQL_RESOWNER_DELETE_START(owner, owner->name);
+
 	/*
 	 * Delete children.  The recursive call will delink the child from me, so
 	 * just iterate as long as there is a child.
@@ -675,6 +745,12 @@ ResourceOwnerDelete(ResourceOwner owner)
 	if (owner->hash)
 		pfree(owner->hash);
 	pfree(owner);
+
+	/*
+	 * Passing the free'd owner pointer here is deliberate, it serves to
+	 * identify the freed owner when concurrently tracing many backends.
+	 */
+	TRACE_POSTGRESQL_RESOWNER_DELETE_DONE((long int)owner, name);
 }
 
 /*
@@ -695,6 +771,8 @@ ResourceOwnerNewParent(ResourceOwner owner,
 {
 	ResourceOwner oldparent = owner->parent;
 
+	TRACE_POSTGRESQL_RESOWNER_NEW_PARENT_START(owner, oldparent, newparent);
+
 	if (oldparent)
 	{
 		if (owner == oldparent->firstchild)
@@ -726,6 +804,8 @@ ResourceOwnerNewParent(ResourceOwner owner,
 		owner->parent = NULL;
 		owner->nextchild = NULL;
 	}
+
+	TRACE_POSTGRESQL_RESOWNER_NEW_PARENT_DONE(owner);
 }
 
 /*
-- 
2.26.2

---

From: Michael Paquier <[email-masked]>
Date: Thu, 26 Nov 2020 16:29:47 +0900

> [off-list for now]

Looks like you have missed something here.
--
Michael

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Thu, 26 Nov 2020 17:52:53 +0900 (JST)

At Thu, 19 Nov 2020 17:27:18 +0800, Julien Rouhaud <[email-masked]> wrote in 
> +1, and +1 for this refactoring.

+1 for making the interface more generic. I thought a similar thing
when I added an resowner array for WaitEventSet (not committed yet).
About performance, though I'm not sure about, there's no reason not to
do this as far as the resowner mechanism doesn't get slower, and +2 if
gets faster.

:p

Coundn't that missing be avoided by just not incrementing i if found?

+		/*

If I read this patch correctly, ResourceOwnerForget doesn't seem to do
such a thing for hash?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 16 Dec 2020 17:46:33 +0200

> Coundn't that missing be avoided by just not incrementing i if found?

Hmm, perhaps. ResourceOwnerForget() can move an entry from the end of 
the array to the beginning, but if it's removing the entry that we're 
just looking at, it probably can't move anything before that entry. I'm 
not very comfortable with that, though. What if the callback releases 
something else as a side effect?

This isn't super-critical for performance, and given that the array is 
very small, it's very cheap to loop through it. So I'm inclined to keep 
it safe.

> If I read this patch correctly, ResourceOwnerForget doesn't seem to do

Hmm, true. I tried removing the loop and hit another issue, however: if 
the same resource has been remembered twice in the same resource owner, 
the callback might remove different reference to it than what we're 
looking at. So we need to loop anyway to handle that. Also, what if the 
callback remembers some other resource in the same resowner, causing the 
hash to grow? I'm not sure if any of the callbacks currently do that, 
but better safe than sorry. I updated the code and the comments accordingly.

Here's an updated version of this patch. I fixed the bit rot, and 
addressed all the other comment issues and such that people pointed out 
(thanks!).

TODO:

1. Performance testing. We discussed this a little bit, but I haven't 
done any more testing.

2. Before this patch, the ResourceOwnerEnlarge() calls enlarged the 
array for the particular "kind" of resource, but now they are all stored 
in the same structure. That could lead to trouble if we do something 
like this:

ResourceOwnerEnlargeAAA()
ResourceOwnerEnlargeBBB()
ResourceOwnerRememberAAA()
ResourceOwnerRememberBBB()

Previously, this was OK, because resources AAA and BBB were kept in 
separate arrays. But after this patch, it's not guaranteed that the 
ResourceOwnerRememberBBB() will find an empty slot.

I don't think we do that currently, but to be sure, I'm planning to grep 
ResourceOwnerRemember and look at each call carefullly. And perhaps we 
can add an assertion for this, although I'm not sure where.

- Heikki

---

From: "[email-masked]" <[email-masked]>
Date: Wed, 13 Jan 2021 01:55:15 +0000

Dear Heikki, 

I'm also interested in this patch, but it cannot be applied to the current HEAD...

$ git apply ~/v2-0001-Make-resowners-more-easily-extensible.patch
error: patch failed: src/common/cryptohash_openssl.c:57
error: src/common/cryptohash_openssl.c: patch does not apply
error: patch failed: src/include/utils/resowner_private.h:1
error: src/include/utils/resowner_private.h: patch does not apply

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 13 Jan 2021 09:18:57 +0200

> error: src/include/utils/resowner_private.h: patch does not apply

Here's a rebased version. Thanks!

- Heikki

---

From: Michael Paquier <[email-masked]>
Date: Wed, 13 Jan 2021 16:22:36 +0900

On Wed, Jan 13, 2021 at 09:18:57AM +0200, Heikki Linnakangas wrote:
> --- a/src/common/cryptohash_openssl.c
> +++ b/src/common/cryptohash_openssl.c
> +static ResourceOwnerFuncs cryptohash_funcs =
> +{
> +	.ReleaseResource = ResOwnerReleaseCryptoHash,
> +	.PrintLeakWarning = ResOwnerPrintCryptoHashLeakWarning,
> +};
> +#endif

Looks like a copy-paste error here.
--
Michael

---

From: "[email-masked]" <[email-masked]>
Date: Wed, 13 Jan 2021 07:39:38 +0000

Dear Heikki,

Thank you for rebasing it, I confirmed it can be applied.
I will check the source.

Now I put the very elementary comment.
ResourceOwnerEnlarge(), ResourceOwnerRemember(), and ResourceOwnerForget()
are exported routines.
They should put below L418.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "[email-masked]" <[email-masked]>
Date: Thu, 14 Jan 2021 10:15:35 +0000

Hi,

I put some comments.

Throughout, some components don’t have helper functions.
(e.g. catcache has ResOwnerReleaseCatCache, but tupdesc doesn't.)
I think it should be unified.

[catcache.c]
> +       ResourceOwnerEnlarge(owner);

This function is not needed.

[syscache.c]
> + CatCache *SysCache[SysCacheSize];

Is it right? Compilation is done even if this variable is static...

[fd.c, dsm.c]
In these files helper functions are implemented as the define directive.
Could you explain the reason? For the performance?

> can add an assertion for this, although I'm not sure where.

Indeed, but I think this line works well, isn't it?

> 	Assert(owner->narr < RESOWNER_ARRAY_SIZE);

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 14 Jan 2021 15:34:56 +0200

> I put some comments.

Thanks for the review!

> I think it should be unified.

Hmm. ResOwnerReleaseTupleDesc() does exist, those functions are needed 
for the callbacks. I think you meant the wrappers around 
ResourceOwnerRemember and ResourceOwnerForget, like 
ResourceOwnerRememberCatCacheRef(). I admit those are not fully 
consistent: I didn't bother with the wrapper functions when there is 
only one caller.

> This function is not needed.

Removed.

> Is it right? Compilation is done even if this variable is static...

Fixed. (It was a leftover from when I was playing with Kyotaro's 
"catcachebench" tool from another thread).

> Could you explain the reason? For the performance?

No particular reason. I turned them all into macros for consistency.

>> 	Assert(owner->narr < RESOWNER_ARRAY_SIZE);

That catches cases where you actually overrun the array, but it doesn't 
catch unsafe patterns when there happens to be enough space left in the 
array. For example, if you have code like this:

/* Make sure there's room for one more entry, but remember *two* things */
ResourceOwnerEnlarge();
ResourceOwnerRemember(foo);
ResourceOwnerRemember(bar);

That is not safe, but it would only fail the assertion if the first 
ResourceOwnerRemember() call happens to consume the last remaining slot 
in the array.

I checked all the callers of ResourceOwnerEnlarge() to see if they're 
safe. A couple of places seemed a bit suspicious. I fixed them by moving 
the ResourceOwnerEnlarge() calls closer to the ResourceOwnerRemember() 
calls, so that it's now easier to see that they are correct. See first 
attached patch.

The second patch is an updated version of the main patch, fixing all the 
little things you and Michael pointed out since the last patch version.

I've been working on performance testing too. I'll post more numbers 
later, but preliminary result from some micro-benchmarking suggests that 
the new code is somewhat slower, except in the common case that the 
object to remember and forget fits in the array. When running the 
regression test suite, about 96% of ResourceOwnerForget() calls fit in 
the array. I think that's acceptable.

- Heikki

---

From: "[email-masked]" <[email-masked]>
Date: Fri, 15 Jan 2021 09:10:34 +0000

Dear Heikki,

> consistent: I didn't bother with the wrapper functions when there is 

Yeah, I meant it. And I prefer your policy.

> consistent: I didn't bother with the wrapper functions when there is 

Good job. I confirmed your fixes, and I confirmed it looks fine.
I will check another ResourceOwnerEnlarge() if I have a time.

> I've been working on performance testing too.

I'm looking forward to seeing it.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "[email-masked]" <[email-masked]>
Date: Mon, 18 Jan 2021 07:49:20 +0000

Dear Heikki,

I apologize for sending again.

> I will check another ResourceOwnerEnlarge() if I have a time.

I checked all ResourceOwnerEnlarge() types after applying patch 0001.
(searched by "grep -rI ResourceOwnerEnlarge")
No problem was found.

Hayato Kuroda
FUJITSU LIMITED

---

From: Heikki Linnakangas <[email-masked]>
Date: Mon, 18 Jan 2021 14:22:33 +0200

> (searched by "grep -rI ResourceOwnerEnlarge")

Great, thanks!

Here are more details on the performance testing I did:

Unpatched
----------

postgres=# \i snaptest.sql
  numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
        0 |        1 |         38.2 |         34.8
        0 |        5 |         35.7 |         35.4
        0 |       10 |         37.6 |         37.6
        0 |       60 |         35.4 |         39.2
        0 |       70 |         55.0 |         53.8
        0 |      100 |         48.6 |         48.9
        0 |     1000 |         54.8 |         57.0
        0 |    10000 |         63.9 |         67.1
        9 |       10 |         39.3 |         38.8
        9 |      100 |         44.0 |         42.5
        9 |     1000 |         45.8 |         47.1
        9 |    10000 |         64.2 |         66.8
       65 |       70 |         45.7 |         43.7
       65 |      100 |         42.9 |         43.7
       65 |     1000 |         46.9 |         45.7
       65 |    10000 |         65.0 |         64.5
(16 rows)

Patched
--------

postgres=# \i snaptest.sql
  numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
        0 |        1 |         35.3 |         33.8
        0 |        5 |         34.8 |         34.6
        0 |       10 |         49.8 |         51.4
        0 |       60 |         53.1 |         57.2
        0 |       70 |         53.2 |         59.6
        0 |      100 |         53.1 |         58.2
        0 |     1000 |         63.1 |         69.7
        0 |    10000 |         83.3 |         83.5
        9 |       10 |         35.0 |         35.1
        9 |      100 |         55.4 |         54.1
        9 |     1000 |         56.8 |         60.3
        9 |    10000 |         88.6 |         82.0
       65 |       70 |         36.4 |         35.1
       65 |      100 |         52.4 |         53.0
       65 |     1000 |         55.8 |         59.4
       65 |    10000 |         79.3 |         80.3
(16 rows)

About the test:

Each test call Register/UnregisterSnapshot in a loop. numsnaps is the 
number of snapshots that are registered in each iteration. For exmaple, 
on the second line with numkeep=0 and numnaps=5, each iteration in the 
LIFO test does essentially:

rs[0] = RegisterSnapshot()
rs[1] = RegisterSnapshot()
rs[2] = RegisterSnapshot()
rs[3] = RegisterSnapshot()
rs[4] = RegisterSnapshot()
UnregisterSnapshot(rs[4]);
UnregisterSnapshot(rs[3]);
UnregisterSnapshot(rs[2]);
UnregisterSnapshot(rs[1]);
UnregisterSnapshot(rs[0]);

In the FIFO tests, the Unregister calls are made in reverse order.

When numkeep is non zero, that many snapshots are registered once at the 
beginning of the test, and released only after the test loop. So this 
simulates the scenario that you remember a bunch of resources and hold 
them for a long time, and while holding them, you do many more 
remember/forget calls.

Based on this test, this patch makes things slightly slower overall. 
However, *not* in the cases that matter the most I believe. That's the 
cases where the (numsnaps - numkeep) is small, so that the hot action 
happens in the array and the hashing is not required. In my testing 
earlier, about 95% of the ResourceOwnerRemember calls in the regression 
tests fall into that category.

There are a few simple things we could do speed this up, if needed. A 
different hash function might be cheaper, for example. And we could 
inline the fast paths of the ResourceOwnerEnlarge and 
ResourceOwnerRemember() into the callers. But I didn't do those things 
as part of this patch, because it wouldn't be a fair comparison; we 
could do those with the old implementation too. And unless we really 
need the speed, it's more readable this way.

I'm OK with these results. Any objections?

Attached are the patches again. Same as previous version, except for a 
couple of little comment changes. And a third patch containing the 
source for the performance test.

- Heikki

---

From: Alvaro Herrera <[email-masked]>
Date: Mon, 18 Jan 2021 11:34:43 -0300

> +	.PrintLeakWarning = ResOwnerPrintJitContextLeakWarning

I think you mean jit_resowner_funcs here; "jit_funcs" is a bit
excessively vague.  Also, why did you choose not to define
ResourceOwnerRememberJIT?  You do that in other modules and it seems
better.

> +	.PrintLeakWarning = ResOwnerPrintCatCacheListLeakWarning

Similar naming issue here, I say.

> diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c
> index 551ec392b60..642e72d8c0f 100644
> @@ -60,6 +59,21 @@ struct pg_cryptohash_ctx
>  #endif
>  };
>  
> +#ifndef FRONTEND
> +static void ResOwnerReleaseCryptoHash(Datum res);
> +static void ResOwnerPrintCryptoHashLeakWarning(Datum res);

The comment is wrong -- "Crypohashes", not "JitContexts".

So according to your performance benchmark, we're willing to accept a
30% performance loss on an allegedly common operation -- numkeep=0
numsnaps=10 becomes 49.8ns from 37.6ns.  That seems a bit shocking.
Maybe you can claim that these operations aren't exactly hot spots, and
so the fact that we remain in the same power-of-ten is sufficient.  Is
that the argument?

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"The Postgresql hackers have what I call a "NASA space shot" mentality.
 Quite refreshing in a world of "weekend drag racer" developers."
(Scott Marlowe)

---

From: Heikki Linnakangas <[email-masked]>
Date: Mon, 18 Jan 2021 17:19:04 +0200

> so the fact that we remain in the same power-of-ten is sufficient.  Is

That's right. The fast path is fast, and that's important. The slow path 
becomes 30% slower, but that's acceptable.

- Heikki

---

From: Robert Haas <[email-masked]>
Date: Mon, 18 Jan 2021 11:11:10 -0500

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Robert Haas <[email-masked]>
Date: Mon, 18 Jan 2021 11:11:57 -0500

> > becomes 30% slower, but that's acceptable.

Sorry for the empty message.

I don't know whether a 30% slowdown will hurt anybody, but it seems
like kind of a lot, and I'm not sure I understand what corresponding
benefit we're getting.

---

From: Michael Paquier <[email-masked]>
Date: Tue, 19 Jan 2021 15:48:08 +0900

[...]
> +/* ResourceOwner callbacks to hold JitContexts  */

Slight copy-paste error here.

>  	/*
>  	 * Ensure, while the spinlock's not yet held, that there's a free
> -	 * refcount entry.
> +	 * refcount entry and that the resoure owner has room to remember the
> +	 * pin.
s/resoure/resource/.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 19 Jan 2021 11:09:16 +0200

> like kind of a lot, and I'm not sure I understand what corresponding

The benefit is to make it easy for extensions to use resource owners to 
track whatever resources they need to track. And arguably, the new 
mechanism is nicer for built-in code, too.

I'll see if I can optimize the slow paths, to make it more palatable.

- Heikki

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 19 Jan 2021 16:45:57 +0200

> ResourceOwnerRememberJIT?  You do that in other modules and it seems

I did it in modules that had more than one ResourceOwnerRemeber/Forget 
call. Didn't seem worth it in functions like IncrTupleDescRefCount(), 
for example.

Hayato Kuroda also pointed that out, though. So perhaps it's better to 
be consistent, to avoid the confusion. I'll add the missing wrappers.

- Heikki

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 21 Jan 2021 00:11:37 +0200

> I'll see if I can optimize the slow paths, to make it more palatable.

Ok, here's a new set of patches, and new test results. I replaced the 
hash function with a cheaper one. I also added the missing wrappers that 
Alvaro and Hayato Kuroda commented on, and fixed the typos that Michael 
Paquier pointed out.

In the test script, I increased the number of iterations used in the 
tests, to make them run longer and produce more stable results. There is 
still a fair amount of jitter in the results, so take any particular 
number with a grain of salt, but the overall trend is repeatable.

The results now look like this:

Unpatched
---------

        0 |        1 |         32.7 |         32.3
        0 |        5 |         33.0 |         32.9
        0 |       10 |         34.1 |         35.5
        0 |       60 |         32.5 |         38.3
        0 |       70 |         47.6 |         48.9
        0 |      100 |         47.9 |         49.3
        0 |     1000 |         52.9 |         52.7
        0 |    10000 |         61.7 |         62.4
        9 |       10 |         38.4 |         37.6
        9 |      100 |         42.3 |         42.3
        9 |     1000 |         43.9 |         45.0
        9 |    10000 |         62.2 |         62.5
       65 |       70 |         42.4 |         42.9
       65 |      100 |         43.2 |         43.9
       65 |     1000 |         44.0 |         45.1
       65 |    10000 |         62.4 |         62.6
(16 rows)

Patched
-------

        0 |        1 |         32.8 |         34.2
        0 |        5 |         33.8 |         32.2
        0 |       10 |         37.2 |         40.2
        0 |       60 |         40.2 |         45.1
        0 |       70 |         40.9 |         48.4
        0 |      100 |         41.9 |         45.2
        0 |     1000 |         49.0 |         55.6
        0 |    10000 |         62.4 |         67.2
        9 |       10 |         33.6 |         33.0
        9 |      100 |         39.6 |         39.7
        9 |     1000 |         42.2 |         45.0
        9 |    10000 |         60.7 |         63.9
       65 |       70 |         32.5 |         33.6
       65 |      100 |         37.5 |         39.7
       65 |     1000 |         42.3 |         46.3
       65 |    10000 |         61.9 |         64.8
(16 rows)

For easier side-by-side comparison, here are the same numbers with the 
patched and unpatched results side by side, and their ratio (ratio < 1 
means the patched version is faster):

LIFO tests:
  numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
        0 |        1 |      32.7 |    32.8 |  1.00
        0 |        5 |      33.0 |    33.8 |  1.02
        0 |       10 |      34.1 |    37.2 |  1.09
        0 |       60 |      32.5 |    40.2 |  1.24
        0 |       70 |      47.6 |    40.9 |  0.86
        0 |      100 |      47.9 |    41.9 |  0.87
        0 |     1000 |      52.9 |    49.0 |  0.93
        0 |    10000 |      61.7 |    62.4 |  1.01
        9 |       10 |      38.4 |    33.6 |  0.88
        9 |      100 |      42.3 |    39.6 |  0.94
        9 |     1000 |      43.9 |    42.2 |  0.96
        9 |    10000 |      62.2 |    60.7 |  0.98
       65 |       70 |      42.4 |    32.5 |  0.77
       65 |      100 |      43.2 |    37.5 |  0.87
       65 |     1000 |      44.0 |    42.3 |  0.96
       65 |    10000 |      62.4 |    61.9 |  0.99
(16 rows)

FIFO tests:
  numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
        0 |        1 |      32.3 |    34.2 |  1.06
        0 |        5 |      32.9 |    32.2 |  0.98
        0 |       10 |      35.5 |    40.2 |  1.13
        0 |       60 |      38.3 |    45.1 |  1.18
        0 |       70 |      48.9 |    48.4 |  0.99
        0 |      100 |      49.3 |    45.2 |  0.92
        0 |     1000 |      52.7 |    55.6 |  1.06
        0 |    10000 |      62.4 |    67.2 |  1.08
        9 |       10 |      37.6 |    33.0 |  0.88
        9 |      100 |      42.3 |    39.7 |  0.94
        9 |     1000 |      45.0 |    45.0 |  1.00
        9 |    10000 |      62.5 |    63.9 |  1.02
       65 |       70 |      42.9 |    33.6 |  0.78
       65 |      100 |      43.9 |    39.7 |  0.90
       65 |     1000 |      45.1 |    46.3 |  1.03
       65 |    10000 |      62.6 |    64.8 |  1.04
(16 rows)

Summary: In the the worst scenario, the patched version is still 24% 
slower than unpatched. But many other scenarios are now faster with the 
patch.

- Heikki

---

From: "[email-masked]" <[email-masked]>
Date: Thu, 21 Jan 2021 02:22:20 +0000

Dear Heikki,

I tested in the same situation, and I confirmed that almost same results are returned.
(results are at the end of the e-mail)

You think that these results are acceptable
because resowners own many resources(more than 64) in general
and it's mainly faster in such a situation, isn't it?

I cannot distinguish correctly, but sounds good.

====test result====

unpatched

 numkeep | numsnaps | lifo_time_ns | fifo_time_ns 
---------+----------+--------------+--------------
       0 |        1 |         68.5 |         69.9
       0 |        5 |         73.2 |         76.8
       0 |       10 |         70.6 |         74.7
       0 |       60 |         68.0 |         75.6
       0 |       70 |         91.3 |         94.8
       0 |      100 |         89.0 |         89.1
       0 |     1000 |         97.9 |         98.9
       0 |    10000 |        116.0 |        115.9
       9 |       10 |         74.7 |         76.6
       9 |      100 |         80.8 |         80.1
       9 |     1000 |         86.0 |         86.2
       9 |    10000 |        116.1 |        116.8
      65 |       70 |         84.7 |         85.3
      65 |      100 |         80.5 |         80.3
      65 |     1000 |         86.3 |         86.2
      65 |    10000 |        115.4 |        115.9
(16 rows)

patched

 numkeep | numsnaps | lifo_time_ns | fifo_time_ns 
---------+----------+--------------+--------------
       0 |        1 |         62.4 |         62.6
       0 |        5 |         68.0 |         66.9
       0 |       10 |         73.6 |         78.1
       0 |       60 |         82.3 |         87.2
       0 |       70 |         83.0 |         89.1
       0 |      100 |         82.8 |         87.9
       0 |     1000 |         88.2 |         96.6
       0 |    10000 |        119.6 |        124.5
       9 |       10 |         62.0 |         62.8
       9 |      100 |         75.3 |         78.0
       9 |     1000 |         82.6 |         89.3
       9 |    10000 |        116.6 |        122.6
      65 |       70 |         66.7 |         66.4
      65 |      100 |         74.6 |         77.2
      65 |     1000 |         82.1 |         88.2
      65 |    10000 |        118.0 |        124.1
(16 rows)

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Michael Paquier <[email-masked]>
Date: Thu, 21 Jan 2021 13:14:12 +0900

On Thu, Jan 21, 2021 at 12:11:37AM +0200, Heikki Linnakangas wrote:
> Summary: In the the worst scenario, the patched version is still 24% slower
> than unpatched. But many other scenarios are now faster with the patch.

Is there a reason explaining the sudden drop for numsnaps within the
[10,60] range?  The gap looks deeper with a low numkeep.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 21 Jan 2021 12:14:10 +0200

> [10,60] range?  The gap looks deeper with a low numkeep.

It's the switch from array to hash table. With the patch, the array 
holds 8 entries. Without the patch, it's 64 entries. So you see a drop 
around those points. I added more data points in that range to get a 
better picture:

LIFO tests:
  numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
        0 |        1 |      32.8 |    32.9 |  1.00
        0 |        2 |      31.6 |    32.8 |  1.04
        0 |        4 |      32.7 |    32.0 |  0.98
        0 |        6 |      34.1 |    33.9 |  0.99
        0 |        8 |      35.1 |    32.4 |  0.92
        0 |       10 |      34.0 |    37.1 |  1.09
        0 |       15 |      33.1 |    35.9 |  1.08
        0 |       20 |      33.0 |    38.8 |  1.18
        0 |       25 |      32.9 |    42.3 |  1.29
        0 |       30 |      32.9 |    40.5 |  1.23
        0 |       35 |      33.1 |    39.9 |  1.21
        0 |       40 |      33.0 |    39.0 |  1.18
        0 |       45 |      35.3 |    41.1 |  1.16
        0 |       50 |      33.0 |    40.8 |  1.24
        0 |       55 |      32.8 |    40.6 |  1.24
        0 |       58 |      33.0 |    41.5 |  1.26
        0 |       60 |      33.1 |    41.6 |  1.26
        0 |       62 |      32.8 |    41.7 |  1.27
        0 |       64 |      46.8 |    40.9 |  0.87
        0 |       66 |      47.0 |    42.5 |  0.90
        0 |       68 |      47.1 |    41.8 |  0.89
        0 |       70 |      47.8 |    41.7 |  0.87
(22 rows)

FIFO tests:
  numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
        0 |        1 |      32.3 |    32.1 |  0.99
        0 |        2 |      33.4 |    31.6 |  0.95
        0 |        4 |      34.0 |    31.4 |  0.92
        0 |        6 |      35.4 |    33.2 |  0.94
        0 |        8 |      34.8 |    31.9 |  0.92
        0 |       10 |      35.4 |    40.2 |  1.14
        0 |       15 |      35.4 |    40.3 |  1.14
        0 |       20 |      35.6 |    43.8 |  1.23
        0 |       25 |      35.4 |    42.4 |  1.20
        0 |       30 |      36.0 |    43.3 |  1.20
        0 |       35 |      36.4 |    45.1 |  1.24
        0 |       40 |      36.9 |    46.6 |  1.26
        0 |       45 |      37.7 |    45.3 |  1.20
        0 |       50 |      37.2 |    43.9 |  1.18
        0 |       55 |      38.4 |    46.8 |  1.22
        0 |       58 |      37.6 |    45.0 |  1.20
        0 |       60 |      37.7 |    46.6 |  1.24
        0 |       62 |      38.4 |    46.5 |  1.21
        0 |       64 |      48.7 |    47.6 |  0.98
        0 |       66 |      48.2 |    45.8 |  0.95
        0 |       68 |      48.5 |    47.5 |  0.98
        0 |       70 |      48.4 |    47.3 |  0.98
(22 rows)

Let's recap the behavior:

Without patch
-------------

For each different kind of resource, there's an array that holds up to 
64 entries. In ResourceOwnerForget(), the array is scanned linearly. If 
the array fills up, we replace the array with a hash table. After 
switching, all operations use the hash table.

With patch
----------

There is one array that holds up to 8 entries. It is shared by all 
resources. In ResourceOwnerForget(), the array is always scanned.

If the array fills up, all the entries are moved to a hash table, 
freeing up space in the array, and the new entry is added to the array. 
So the array is used together with the hash table, like an LRU cache of 
the most recently remembered entries.

Why this change? I was afraid that now that all different resources 
share the same data structure, remembering e.g. a lot of locks at the 
beginning of a transaction would cause the switch to the hash table, 
making all subsequent remember/forget operations, like for buffer pins, 
slower. That kind of interference seems bad. By continuing to use the 
array for the recently-remembered entries, we avoid that problem. The 
[numkeep, numsnaps] = [65, 70] test is in that regime, and the patched 
version was significantly faster.

Because the array is now always scanned, I felt that it needs to be 
small, to avoid wasting much time scanning for entries that have already 
been moved to the hash table. That's why I made it just 8 entries.

Perhaps 8 entries is too small, after all? Linearly scanning an array is 
very fast. To test that, I bumped up RESOWNER_ARRAY_SIZE to 64, and ran 
the test again:

        0 |        1 |         35.4 |         31.5
        0 |        2 |         32.3 |         32.3
        0 |        4 |         32.8 |         31.0
        0 |        6 |         34.5 |         33.7
        0 |        8 |         33.9 |         32.7
        0 |       10 |         33.7 |         33.0
        0 |       15 |         34.8 |         33.1
        0 |       20 |         35.0 |         32.6
        0 |       25 |         36.9 |         33.0
        0 |       30 |         38.7 |         33.2
        0 |       35 |         39.9 |         34.5
        0 |       40 |         40.8 |         35.5
        0 |       45 |         42.6 |         36.4
        0 |       50 |         44.9 |         37.8
        0 |       55 |         45.4 |         38.5
        0 |       58 |         47.7 |         39.6
        0 |       60 |         45.9 |         40.2
        0 |       62 |         47.6 |         40.9
        0 |       64 |         51.4 |         41.2
        0 |       66 |         38.2 |         39.8
        0 |       68 |         39.7 |         40.3
        0 |       70 |         38.1 |         40.9
(22 rows)

Here you can see that as numsnaps increases, the test becomes slower, 
but then it becomes faster again at 64-66, when it switches to the hash 
table. So 64 seems too much. 32 seems to be the sweet spot here, that's 
where scanning the hash and scanning the array are about the same speed.

- Heikki

---

From: Robert Haas <[email-masked]>
Date: Mon, 25 Jan 2021 12:14:42 -0500

> where scanning the hash and scanning the array are about the same speed.

That sounds good. I mean, it could be that not all hardware behaves
the same here. But trying to get it into the right ballpark makes
sense.

I also like the fact that this now has some cases where it wins by a
significant margin. That's pretty cool; thanks for working on it!

---

From: Ibrar Ahmed <[email-masked]>
Date: Mon, 8 Mar 2021 21:47:42 +0500

> significant margin. That's pretty cool; thanks for working on it!
The patchset does not apply successfully, there are some hunk failures.

http://cfbot.cputube.org/patch_32_2834.log

v6-0002-Make-resowners-more-easily-extensible.patch

1 out of 6 hunks FAILED -- saving rejects to file
src/backend/utils/cache/plancache.c.rej
2 out of 15 hunks FAILED -- saving rejects to file
src/backend/utils/resowner/resowner.c.rej

Can we get a rebase?

I am marking the patch "Waiting on Author"

-- 
Ibrar Ahmed

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 9 Mar 2021 14:40:42 +0200

On 08/03/2021 18:47, Ibrar Ahmed wrote:
> <http://cfbot.cputube.org/patch_32_2834.log>
> 

Here you go.

- Heikki

---

From: vignesh C <[email-masked]>
Date: Wed, 14 Jul 2021 17:44:42 +0530

> > src/backend/utils/resowner/resowner.c.rej

The patch does not apply on Head anymore, could you rebase and post a
patch. I'm changing the status to "Waiting for Author".

Regards,
Vignesh

---

From: Alvaro Herrera <[email-masked]>
Date: Wed, 14 Jul 2021 10:07:07 -0400

> patch. I'm changing the status to "Waiting for Author".

Support for hmac was added by e6bdfd9700eb so the rebase is not trivial.

-- 
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/
"Ellos andaban todos desnudos como su madre los parió, y también las mujeres,
aunque no vi más que una, harto moza, y todos los que yo vi eran todos
mancebos, que ninguno vi de edad de más de XXX años" (Cristóbal Colón)

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 14 Jul 2021 17:40:27 +0300

> Support for hmac was added by e6bdfd9700eb so the rebase is not trivial.

Yeah, needed some manual fixing, but here you go.

- Heikki

---

From: Zhihong Yu <[email-masked]>
Date: Wed, 14 Jul 2021 08:18:52 -0700

> Yeah, needed some manual fixing, but here you go.
Hi,
For the loop over the hash:

+       for (int idx = 0; idx < capacity; idx++)
        {
-           if (olditemsarr[i] != resarr->invalidval)
-               ResourceArrayAdd(resarr, olditemsarr[i]);
+           while (owner->hash[idx].kind != NULL &&
+                  owner->hash[idx].kind->phase == phase)
...
+   } while (capacity != owner->capacity);

Since the phase variable doesn't seem to change for the while loop, I
wonder what benefit the while loop has (since the release is governed by
phase).

Cheers

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 14 Jul 2021 18:26:13 +0300

Thanks for having a look!

> wonder what benefit the while loop has (since the release is governed by 

Hmm, the phase variable doesn't change, but could the element at 
'owner->hash[idx]' change? I'm not sure about that. The loop is supposed 
to handle the case that the hash table grows; could that replace the 
element at 'owner->hash[idx]' with something else, with different phase? 
The check is very cheap, so I'm inclined to keep it to be sure.

- Heikki

---

From: Zhihong Yu <[email-masked]>
Date: Wed, 14 Jul 2021 08:41:43 -0700

> The check is very cheap, so I'm inclined to keep it to be sure.
Hi,
Agreed that ```owner->hash[idx].kind->phase == phase``` can be kept.

I just wonder if we should put limit on the number of iterations for the
while loop.
Maybe add a bool variable indicating that kind->ReleaseResource(value) is
called in the inner loop.
If there is no releasing when the inner loop finishes, we can come out of
the while loop.

Cheers

---

From: Aleksander Alekseev <[email-masked]>
Date: Wed, 8 Sep 2021 13:19:19 +0300

Hi Heikki,

> Yeah, needed some manual fixing, but here you go.

Thanks for working on this!

v8-0002 didn't apply to the current master, so I rebased it. See
attached v9-* patches. I also included v9-0004 with some minor tweaks
from me. I have several notes regarding the code.

1. Not sure if I understand this code of ResourceOwnerReleaseAll():
```
    /* First handle all the entries in the array. */
    do
    {
        found = false;
        for (int i = 0; i < owner->narr; i++)
        {
            if (owner->arr[i].kind->phase == phase)
            {
                Datum        value = owner->arr[i].item;
                ResourceOwnerFuncs *kind = owner->arr[i].kind;

                if (printLeakWarnings)
                    kind->PrintLeakWarning(value);
                kind->ReleaseResource(value);
                found = true;
            }
        }

        /*
         * If any resources were released, check again because some of the
         * elements might have been moved by the callbacks. We don't want to
         * miss them.
         */
    } while (found && owner->narr > 0);
```

Shouldn't we mark the resource as released and/or decrease narr and/or
save the last processed i? Why this will not call ReleaseResource()
endlessly on the same resource (array item)? Same question for the
following code that iterates over the hash table.

2. Just an idea/observation. It's possible that the performance of
ResourceOwnerEnlarge() can be slightly improved. Since the size of the
hash table is always a power of 2 and the code always doubles the size
of the hash table, (idx & mask) value will get one extra bit, which
can be 0 or 1. If it's 0, the value is already in its place,
otherwise, it should be moved on the known distance. In other words,
it's possible to copy `oldhash` to `newhash` and then move only half
of the items. I don't claim that this code necessarily should be
faster, or that this should be checked in the scope of this work.

-- 
Best regards,
Aleksander Alekseev

---

From: Michael Paquier <[email-masked]>
Date: Fri, 1 Oct 2021 16:18:31 +0900

> Thanks for working on this!

The latest patch does not apply anymore, and has been waiting on
author for a couple of weeks now, so switched as RwF for now.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Mon, 18 Oct 2021 14:17:30 +0300

> from me. I have several notes regarding the code.

Thanks!

Attached is a newly rebased version. It includes your tweaks, and a few 
more comment and indentation tweaks.

> following code that iterates over the hash table.

ReleaseResource() must call ResourceOwnerForget(), which removes the 
item from the or hash table. This works the same as the code in 'master':

> 		/*
> 		 * Release buffer pins.  Note that ReleaseBuffer will remove the
> 		 * buffer entry from our array, so we just have to iterate till there
> 		 * are none.
> 		 *
> 		 * During a commit, there shouldn't be any remaining pins --- that
> 		while (ResourceArrayGetAny(&(owner->bufferarr), &foundres))
> 		{
> 			Buffer		res = DatumGetBuffer(foundres);
> 
> 			if (isCommit)
> 				PrintBufferLeakWarning(res);
> 			ReleaseBuffer(res);
> 		}

That comment explains how it works. I added a comment like that in this 
patch, too.

> faster, or that this should be checked in the scope of this work.

Hmm, the hash table uses open addressing, I think we want to also 
rearrange any existing entries that might not be at their right place 
because of collisions. We don't actually do that currently when an 
element is removed (even before this patch), but enlarging the array is 
a good opportunity for it. In any case, I haven't seen 
ResourceOwnerEnlarge() show up while profiling, so I'm going to leave it 
as it is.

- Heikki

---

From: Aleksander Alekseev <[email-masked]>
Date: Tue, 23 Nov 2021 18:37:03 +0300

Hi Heikki,

> more comment and indentation tweaks.

v10-0002 rotted a little so I rebased it. The new patch set passed `make
installcheck-world` locally, but let's see if cfbot has a second opinion.

I'm a bit busy with various things at the moment, so (hopefully) I will
submit the actual code review in the follow-up email.

-- 
Best regards,
Aleksander Alekseev

---

From: Aleksander Alekseev <[email-masked]>
Date: Fri, 26 Nov 2021 15:40:57 +0300

Hi Heikki,

> I will submit the actual code review in the follow-up email

The patchset is in a good shape. I'm changing the status to "Ready for
Committer".

-- 
Best regards,
Aleksander Alekseev

---

From: Heikki Linnakangas <[email-masked]>
Date: Mon, 31 Oct 2022 10:51:36 +0100

On 12/01/2022 07:57, Julien Rouhaud wrote:
> On Fri, Nov 26, 2021 at 8:41 PM Aleksander Alekseev
> <[email-masked]> wrote:
>>
> The 2nd patch doesn't apply anymore due to a conflict on
> resowner_private.h: http://cfbot.cputube.org/patch_36_3364.log.

Rebased version attached. Given that Aleksander marked this as Ready for 
Committer earlier, I'll add this to the next commitfest in that state, 
and will commit in the next few days, barring any new objections.

- Heikki
From 14d05ab81a3615a0772bf34297fafce406e160ce Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Mon, 31 Oct 2022 10:33:36 +0100
Subject: [PATCH v12 1/3] Move a few ResourceOwnerEnlarge() calls for safety
 and clarity.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These are functions where quite a lot of things happen between the
ResourceOwnerEnlarge and ResourceOwnerRemember calls. It's important that
there are no unrelated ResourceOwnerRemember() calls in the code in
between, otherwise the entry reserved by the ResourceOwnerEnlarge() call
might be used up by the intervening ResourceOwnerRemember() and not be
available at the intended ResourceOwnerRemember() call anymore. The longer
the code path between them is, the harder it is to verify that.

In bufmgr.c, there is a function similar to ResourceOwnerEnlarge(), to
ensure that the private refcount array has enough space. The
ReservePrivateRefCountEntry() calls, analogous to ResourceOwnerEnlarge(),
were made at different places than the ResourceOwnerEnlarge() calls. Move
the ResourceOwnerEnlarge() calls together with the
ReservePrivateRefCountEntry() calls for consistency.

Reviewed-by: Aleksander Alekseev, Michael Paquier, Julien Rouhaud, Kyotaro Horiguchi, Hayato Kuroda, Álvaro Herrera, Zhihong Yu
Discussion: https://www.postgresql.org/message-id/cbfabeb0-cd3c-e951-a572-19b365ed314d%40iki.fi
---
 src/backend/storage/buffer/bufmgr.c   | 39 +++++++++++----------------
 src/backend/storage/buffer/localbuf.c |  3 +++
 src/backend/utils/cache/catcache.c    | 13 ++++++---
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 73d30bf6191..d12e81097c5 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -828,9 +828,6 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 
 	*hit = false;
 
-	/* Make sure we will have room to remember the buffer pin */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	isExtend = (blockNum == P_NEW);
 
 	TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum,
@@ -1192,9 +1189,11 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	{
 		/*
 		 * Ensure, while the spinlock's not yet held, that there's a free
-		 * refcount entry.
+		 * refcount entry and that the resource owner has room to remember the
+		 * pin.
 		 */
 		ReservePrivateRefCountEntry();
+		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
 		/*
 		 * Select a victim buffer.  The buffer is returned with its header
@@ -1695,8 +1694,6 @@ ReleaseAndReadBuffer(Buffer buffer,
  * taking the buffer header lock; instead update the state variable in loop of
  * CAS operations. Hopefully it's just a single CAS.
  *
- * Note that ResourceOwnerEnlargeBuffers must have been done already.
- *
  * Returns true if buffer is BM_VALID, else false.  This provision allows
  * some callers to avoid an extra spinlock cycle.
  */
@@ -1707,6 +1704,8 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy)
 	bool		result;
 	PrivateRefCountEntry *ref;
 
+	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+
 	ref = GetPrivateRefCountEntry(b, true);
 
 	if (ref == NULL)
@@ -1787,7 +1786,8 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy)
  * The spinlock is released before return.
  *
  * As this function is called with the spinlock held, the caller has to
- * previously call ReservePrivateRefCountEntry().
+ * previously call ReservePrivateRefCountEntry() and
+ * ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
  *
  * Currently, no callers of this function want to modify the buffer's
  * usage_count at all, so there's no need for a strategy parameter.
@@ -1960,9 +1960,6 @@ BufferSync(int flags)
 	int			mask = BM_DIRTY;
 	WritebackContext wb_context;
 
-	/* Make sure we can handle the pin inside SyncOneBuffer */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	/*
 	 * Unless this is a shutdown checkpoint or we have been explicitly told,
 	 * we write only permanent, dirty buffers.  But at shutdown or end of
@@ -2436,9 +2433,6 @@ BgBufferSync(WritebackContext *wb_context)
 	 * requirements, or hit the bgwriter_lru_maxpages limit.
 	 */
 
-	/* Make sure we can handle the pin inside SyncOneBuffer */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	num_to_scan = bufs_to_lap;
 	num_written = 0;
 	reusable_buffers = reusable_buffers_est;
@@ -2520,8 +2514,6 @@ BgBufferSync(WritebackContext *wb_context)
  *
  * (BUF_WRITTEN could be set in error if FlushBuffer finds the buffer clean
  * after locking it, but we don't care all that much.)
- *
- * Note: caller must have done ResourceOwnerEnlargeBuffers.
  */
 static int
 SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context)
@@ -2531,7 +2523,9 @@ SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context)
 	uint32		buf_state;
 	BufferTag	tag;
 
+	/* Make sure we can handle the pin */
 	ReservePrivateRefCountEntry();
+	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
 	/*
 	 * Check whether buffer needs writing.
@@ -3562,9 +3556,6 @@ FlushRelationBuffers(Relation rel)
 		return;
 	}
 
-	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	for (i = 0; i < NBuffers; i++)
 	{
 		uint32		buf_state;
@@ -3578,7 +3569,9 @@ FlushRelationBuffers(Relation rel)
 		if (!BufTagMatchesRelFileLocator(&bufHdr->tag, &rel->rd_locator))
 			continue;
 
+		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
+		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (BufTagMatchesRelFileLocator(&bufHdr->tag, &rel->rd_locator) &&
@@ -3635,9 +3628,6 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 	if (use_bsearch)
 		pg_qsort(srels, nrels, sizeof(SMgrSortArray), rlocator_comparator);
 
-	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	for (i = 0; i < NBuffers; i++)
 	{
 		SMgrSortArray *srelent = NULL;
@@ -3676,7 +3666,9 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		if (srelent == NULL)
 			continue;
 
+		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
+		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (BufTagMatchesRelFileLocator(&bufHdr->tag, &srelent->rlocator) &&
@@ -3870,9 +3862,6 @@ FlushDatabaseBuffers(Oid dbid)
 	int			i;
 	BufferDesc *bufHdr;
 
-	/* Make sure we can handle the pin inside the loop */
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
-
 	for (i = 0; i < NBuffers; i++)
 	{
 		uint32		buf_state;
@@ -3886,7 +3875,9 @@ FlushDatabaseBuffers(Oid dbid)
 		if (bufHdr->tag.dbOid != dbid)
 			continue;
 
+		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
+		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (bufHdr->tag.dbOid == dbid &&
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 30d67d1c40d..58e7c32c1e3 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -123,6 +123,9 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
 	if (LocalBufHash == NULL)
 		InitLocalBuffers();
 
+	/* Make sure we will have room to remember the buffer pin */
+	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+
 	/* See if the desired buffer already exists */
 	hresult = (LocalBufferLookupEnt *)
 		hash_search(LocalBufHash, (void *) &newTag, HASH_FIND, NULL);
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 38e943fab2b..e7b5b20ea29 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1604,8 +1604,6 @@ SearchCatCacheList(CatCache *cache,
 	 * block to ensure we can undo those refcounts if we get an error before
 	 * we finish constructing the CatCList.
 	 */
-	ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
-
 	ctlist = NIL;
 
 	PG_TRY();
@@ -1693,13 +1691,22 @@ SearchCatCacheList(CatCache *cache,
 
 		table_close(relation, AccessShareLock);
 
+		/* Make sure the resource owner has room to remember this entry. */
+		ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
+
 		/* Now we can build the CatCList entry. */
 		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
 		nmembers = list_length(ctlist);
 		cl = (CatCList *)
 			palloc(offsetof(CatCList, members) + nmembers * sizeof(CatCTup *));
 
-		/* Extract key values */
+		/*
+		 * Extract key values.
+		 *
+		 * XXX: If we run out of memory while copying the key values, we will
+		 * leak any allocations we had already made in the CacheMemoryContext.
+		 * That is unlikely enough that we just accept the risk.
+		 */
 		CatCacheCopyKeys(cache->cc_tupdesc, nkeys, cache->cc_keyno,
 						 arguments, cl->keys);
 		MemoryContextSwitchTo(oldcxt);
-- 
2.30.2

From e0701d958b1fb55ae0d0cea79221ca1653ee0182 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Mon, 31 Oct 2022 10:48:57 +0100
Subject: [PATCH v12 2/3] Make resowners more easily extensible.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Instead of having a separate array/hash for each resource kind, use a
single array and hash to hold all kinds of resources. This makes it
possible to introduce new resource "kinds" without having to modify the
ResourceOwnerData struct. In particular, this makes it possible for
extensions to register custom resource kinds.

The old approach was to have a small array of resources of each kind, and
if it fills up, switch to a hash table. The new approach also uses an
array and a hash, but now the array and a hash are used at the same time.
The array is used to hold the recently added resources, and when it fills
up, they are moved to the hash. This keeps the access to recent entries
fast, even when there are a lot of long-held resources.

All the resource-specific ResourceOwnerEnlarge*(), ResourceOwnerRemember*(),
and ResourceOwnerForget*() functions have been replaced with three generic
functions that take resource kind as argument. For convenience, we still
define resource-specific wrapper macros around the generic functions, with
the same old names, but they are now defined in the source files that use
those resource kinds.

Reviewed-by: Aleksander Alekseev, Michael Paquier, Julien Rouhaud, Kyotaro Horiguchi, Hayato Kuroda, Álvaro Herrera, Zhihong Yu
Discussion: https://www.postgresql.org/message-id/cbfabeb0-cd3c-e951-a572-19b365ed314d%40iki.fi
---
 src/backend/access/common/tupdesc.c   |   42 +-
 src/backend/jit/jit.c                 |    2 -
 src/backend/jit/llvm/llvmjit.c        |   46 +-
 src/backend/storage/buffer/bufmgr.c   |   47 +-
 src/backend/storage/buffer/localbuf.c |    4 +-
 src/backend/storage/file/fd.c         |   44 +-
 src/backend/storage/ipc/dsm.c         |   44 +-
 src/backend/storage/lmgr/lock.c       |    2 +-
 src/backend/utils/cache/catcache.c    |   74 +-
 src/backend/utils/cache/plancache.c   |   45 +-
 src/backend/utils/cache/relcache.c    |   41 +-
 src/backend/utils/resowner/README     |   18 +-
 src/backend/utils/resowner/resowner.c | 1327 +++++++------------------
 src/backend/utils/time/snapmgr.c      |   39 +-
 src/common/cryptohash_openssl.c       |   48 +-
 src/common/hmac_openssl.c             |   46 +-
 src/include/storage/buf_internals.h   |    9 +
 src/include/utils/catcache.h          |    3 -
 src/include/utils/plancache.h         |    2 +
 src/include/utils/resowner.h          |   45 +-
 src/include/utils/resowner_private.h  |  112 ---
 src/tools/pgindent/typedefs.list      |    4 +-
 22 files changed, 913 insertions(+), 1131 deletions(-)
 delete mode 100644 src/include/utils/resowner_private.h

index b7f918c877b..9636abb4ff7 100644
@@ -30,9 +30,27 @@
 
+/* ResourceOwner callbacks to hold tupledesc references  */
+static void ResOwnerReleaseTupleDesc(Datum res);
+static void ResOwnerPrintTupleDescLeakWarning(Datum res);
+
+static ResourceOwnerFuncs tupdesc_resowner_funcs =
+{
+	/* relcache references */
+	.name = "tupdesc reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseTupleDesc,
+	.PrintLeakWarning = ResOwnerPrintTupleDescLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberTupleDesc(owner, tupdesc) \
+	ResourceOwnerRemember(owner, PointerGetDatum(tupdesc), &tupdesc_resowner_funcs)
+#define ResourceOwnerForgetTupleDesc(owner, tupdesc) \
+	ResourceOwnerForget(owner, PointerGetDatum(tupdesc), &tupdesc_resowner_funcs)
 
 /*
  * CreateTemplateTupleDesc
@@ -367,7 +385,7 @@ IncrTupleDescRefCount(TupleDesc tupdesc)
 {
 	Assert(tupdesc->tdrefcount >= 0);
 
-	ResourceOwnerEnlargeTupleDescs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	tupdesc->tdrefcount++;
 	ResourceOwnerRememberTupleDesc(CurrentResourceOwner, tupdesc);
 }
@@ -919,3 +937,23 @@ BuildDescFromLists(List *names, List *types, List *typmods, List *collations)
 
 	return desc;
 }
+
+
+/*
index 91a6b2b63ad..9354ba34c3b 100644
 
 /* GUCs */
 bool		jit_enabled = true;

index 9aca7fc7a47..f0e15bc6345 100644
 
@@ -121,8 +121,26 @@ static LLVMOrcLLJITRef llvm_create_jit_instance(LLVMTargetMachineRef tm);
 
-PG_MODULE_MAGIC;
+/* ResourceOwner callbacks to hold JitContexts  */
+static void ResOwnerReleaseJitContext(Datum res);
+static void ResOwnerPrintJitContextLeakWarning(Datum res);
+
+static ResourceOwnerFuncs jit_resowner_funcs =
+{
+	/* relcache references */
+	.name = "LLVM JIT context",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseJitContext,
+	.PrintLeakWarning = ResOwnerPrintJitContextLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberJIT(owner, handle) \
+	ResourceOwnerRemember(owner, PointerGetDatum(handle), &jit_resowner_funcs)
+#define ResourceOwnerForgetJIT(owner, handle) \
+	ResourceOwnerForget(owner, PointerGetDatum(handle), &jit_resowner_funcs)
 
+PG_MODULE_MAGIC;
 
 /*
  * Initialize LLVM JIT provider.
@@ -151,7 +169,7 @@ llvm_create_context(int jitFlags)

 
 	context = MemoryContextAllocZero(TopMemoryContext,
 									 sizeof(LLVMJitContext));
@@ -159,7 +177,7 @@ llvm_create_context(int jitFlags)
 
 	/* ensure cleanup */
 	context->base.resowner = CurrentResourceOwner;
-	ResourceOwnerRememberJIT(CurrentResourceOwner, PointerGetDatum(context));
+	ResourceOwnerRememberJIT(CurrentResourceOwner, context);
 
 	return context;
 }
@@ -221,6 +239,8 @@ llvm_release_context(JitContext *context)
 	}
 	list_free(llvm_context->handles);
 	llvm_context->handles = NIL;
+
+	ResourceOwnerForgetJIT(context->resowner, context);
 }
 
 /*
@@ -1266,3 +1286,21 @@ llvm_error_message(LLVMErrorRef error)
 }
 
+	jit_release_context((JitContext *) DatumGetPointer(res));
+	JitContext *context = (JitContext *) DatumGetPointer(res);
index d12e81097c5..0f0cedc8b39 100644
@@ -54,7 +54,7 @@

@@ -208,6 +208,18 @@ static PrivateRefCountEntry *GetPrivateRefCountEntry(Buffer buffer, bool do_move
 static inline int32 GetPrivateRefCount(Buffer buffer);
 static void ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref);
 
@@ -630,7 +642,7 @@ ReadRecentBuffer(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockN
 
 	Assert(BufferIsValid(recent_buffer));
 
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	ReservePrivateRefCountEntry();
 	InitBufferTag(&tag, &rlocator, forkNum, blockNum);
 
@@ -1193,7 +1205,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 		 * pin.
 		 */
 		ReservePrivateRefCountEntry();
-		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 		/*
 		 * Select a victim buffer.  The buffer is returned with its header
@@ -1704,7 +1716,7 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy)
 	bool		result;
 	PrivateRefCountEntry *ref;

 	ref = GetPrivateRefCountEntry(b, true);
 
@@ -1787,7 +1799,7 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy)
  *
  * As this function is called with the spinlock held, the caller has to
  * previously call ReservePrivateRefCountEntry() and
- * ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+ * ResourceOwnerEnlarge(CurrentResourceOwner);
  *
  * Currently, no callers of this function want to modify the buffer's
  * usage_count at all, so there's no need for a strategy parameter.
@@ -2525,7 +2537,7 @@ SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context)
 
 	/* Make sure we can handle the pin */
 	ReservePrivateRefCountEntry();
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 
 	/*
 	 * Check whether buffer needs writing.
@@ -3571,7 +3583,7 @@ FlushRelationBuffers(Relation rel)
 
 		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
-		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (BufTagMatchesRelFileLocator(&bufHdr->tag, &rel->rd_locator) &&
@@ -3668,7 +3680,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 
 		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
-		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (BufTagMatchesRelFileLocator(&bufHdr->tag, &srelent->rlocator) &&
@@ -3877,7 +3889,7 @@ FlushDatabaseBuffers(Oid dbid)
 
 		/* Make sure we can handle the pin */
 		ReservePrivateRefCountEntry();
-		ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 		buf_state = LockBufHdr(bufHdr);
 		if (bufHdr->tag.dbOid == dbid &&
@@ -3960,7 +3972,7 @@ void
 IncrBufferRefCount(Buffer buffer)
 {
 	Assert(BufferIsPinned(buffer));
-	ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	if (BufferIsLocal(buffer))
 		LocalRefCount[-buffer - 1]++;
 	else
@@ -5021,3 +5033,18 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
 				(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
 				 errmsg("snapshot too old")));
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerReleaseBuffer(Datum res)
+{
+	ReleaseBuffer(DatumGetInt32(res));
+}
+
+static void
+ResOwnerPrintBufferLeakWarning(Datum res)
+{
+	PrintBufferLeakWarning(DatumGetInt32(res));
+}
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 58e7c32c1e3..f83722173ed 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -22,7 +22,7 @@
 #include "storage/bufmgr.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"

 /*#define LBDEBUG*/
@@ -124,7 +124,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
 		InitLocalBuffers();

 	/* See if the desired buffer already exists */
 	hresult = (LocalBufferLookupEnt *)
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 4151cafec54..41a5386f512 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -98,7 +98,7 @@
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "utils/guc.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 
 /* Define PG_FLUSH_DATA_WORKS if we have an implementation for pg_flush_data */
 #if defined(HAVE_SYNC_FILE_RANGE)
@@ -349,6 +349,24 @@ static void unlink_if_exists_fname(const char *fname, bool isdir, int elevel);
 static int	fsync_parent_path(const char *fname, int elevel);

@@ -1453,7 +1471,7 @@ ReportTemporaryFileUsage(const char *path, off_t size)
 
 /*
@@ -1637,7 +1655,7 @@ OpenTemporaryFile(bool interXact)
 
 	/*
 	 * If some temp tablespace(s) have been given to us, try to use the next
@@ -1769,7 +1787,7 @@ PathNameCreateTemporaryFile(const char *path, bool error_on_failure)
 
 	Assert(temporary_files_allowed);	/* check temp file access is up */

 	/*
 	 * Open the file.  Note: we don't use O_EXCL, in case there is an orphaned
@@ -1809,7 +1827,7 @@ PathNameOpenTemporaryFile(const char *path, int mode)
 
 	Assert(temporary_files_allowed);	/* check temp file access is up */

 
@@ -3737,3 +3755,19 @@ data_sync_elevel(int elevel)
 {
index a360325f864..d1eea705ebc 100644
@@ -38,13 +38,15 @@

@@ -140,6 +142,25 @@ static dsm_control_header *dsm_control;
 static Size dsm_control_mapped_size = 0;
 static void *dsm_control_impl_private = NULL;
 
+
@@ -908,7 +929,7 @@ void
@@ -1175,7 +1196,7 @@ dsm_create_descriptor(void)
 	dsm_segment *seg;

@@ -1254,3 +1275,20 @@ is_main_region_dsm_handle(dsm_handle handle)
index 3d1049cf756..bf363001e0a 100644
@@ -48,7 +48,7 @@

index e7b5b20ea29..607ddb6f6f9 100644
@@ -32,12 +32,13 @@

@@ -105,6 +106,42 @@ static void CatCacheCopyKeys(TupleDesc tupdesc, int nkeys, int *attnos,
  *					internal support functions
  */
 
+/* ResourceOwner callbacks to hold catcache references */
+
+static void ResOwnerReleaseCatCache(Datum res);
+static void ResOwnerPrintCatCacheLeakWarning(Datum res);
+static void ResOwnerReleaseCatCacheList(Datum res);
+static void ResOwnerPrintCatCacheListLeakWarning(Datum res);
+
+static ResourceOwnerFuncs catcache_resowner_funcs =
+{
+	/* catcache references */
+	.name = "catcache reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCatCache,
+	.PrintLeakWarning = ResOwnerPrintCatCacheLeakWarning
+};
+
+static ResourceOwnerFuncs catlistref_resowner_funcs =
+{
+	/* catcache-list pins */
+	.name = "catcache list reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCatCacheList,
+	.PrintLeakWarning = ResOwnerPrintCatCacheListLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberCatCacheRef(owner, tuple) \
+	ResourceOwnerRemember(owner, PointerGetDatum(tuple), &catcache_resowner_funcs)
+#define ResourceOwnerForgetCatCacheRef(owner, tuple) \
+	ResourceOwnerForget(owner, PointerGetDatum(tuple), &catcache_resowner_funcs)
+#define ResourceOwnerRememberCatCacheListRef(owner, list) \
+	ResourceOwnerRemember(owner, PointerGetDatum(list), &catlistref_resowner_funcs)
+#define ResourceOwnerForgetCatCacheListRef(owner, list) \
+	ResourceOwnerForget(owner, PointerGetDatum(list), &catlistref_resowner_funcs)
+
+
 /*
  * Hash and equality functions for system types that are used as cache key
  * fields.  In some cases, we just call the regular SQL-callable functions for
@@ -1267,7 +1304,7 @@ SearchCatCacheInternal(CatCache *cache,
 		 */
 		if (!ct->negative)
 		{
-			ResourceOwnerEnlargeCatCacheRefs(CurrentResourceOwner);
+			ResourceOwnerEnlarge(CurrentResourceOwner);
 			ct->refcount++;
 			ResourceOwnerRememberCatCacheRef(CurrentResourceOwner, &ct->tuple);
 
@@ -1368,7 +1405,7 @@ SearchCatCacheMiss(CatCache *cache,
 									 hashValue, hashIndex,
 									 false);
 		/* immediately set the refcount to 1 */
-		ResourceOwnerEnlargeCatCacheRefs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 		ct->refcount++;
 		ResourceOwnerRememberCatCacheRef(CurrentResourceOwner, &ct->tuple);
 		break;					/* assume only one match */
@@ -1580,7 +1617,7 @@ SearchCatCacheList(CatCache *cache,
 		dlist_move_head(&cache->cc_lists, &cl->cache_elem);

@@ -1692,7 +1729,7 @@ SearchCatCacheList(CatCache *cache,
 		table_close(relation, AccessShareLock);
 
 		/* Make sure the resource owner has room to remember this entry. */
-		ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
+		ResourceOwnerEnlarge(CurrentResourceOwner);
 
 		/* Now we can build the CatCList entry. */
 		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
@@ -2064,14 +2101,19 @@ PrepareToInvalidateCacheTuple(Relation relation,
 	}
 }
 
-
 /*
 
@@ -2085,9 +2127,17 @@ PrintCatCacheLeakWarning(HeapTuple tuple)
 		 ct->refcount);
 }
 
-void
index cc943205d34..319256c99a4 100644
@@ -115,6 +115,26 @@ static void PlanCacheRelCallback(Datum arg, Oid relid);
 
+/* ResourceOwner callbacks to track plancache references */
+static void ResOwnerReleaseCachedPlan(Datum res);
+static void ResOwnerPrintPlanCacheLeakWarning(Datum res);
+
+/* this is exported for ResourceOwnerReleaseAllPlanCacheRefs() */
+ResourceOwnerFuncs planref_resowner_funcs =
+{
+	.name = "plancache reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCachedPlan,
+	.PrintLeakWarning = ResOwnerPrintPlanCacheLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberPlanCacheRef(owner, plan) \
+	ResourceOwnerRemember(owner, PointerGetDatum(plan), &planref_resowner_funcs)
+#define ResourceOwnerForgetPlanCacheRef(owner, plan) \
+	ResourceOwnerForget(owner, PointerGetDatum(plan), &planref_resowner_funcs)
+
+
 /* GUC parameter */
 int			plan_cache_mode = PLAN_CACHE_MODE_AUTO;
 
@@ -1229,7 +1249,7 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
 
@@ -1392,7 +1412,7 @@ CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
@@ -1451,7 +1471,7 @@ CachedPlanIsSimplyValid(CachedPlanSource *plansource, CachedPlan *plan,
@@ -2205,3 +2225,20 @@ ResetPlanCache(void)
+	ReleaseCachedPlan((CachedPlan *) DatumGetPointer(res), CurrentResourceOwner);
index bd6cd4e47b5..ab68df1277e 100644
@@ -80,13 +80,14 @@
 
@@ -2115,6 +2116,24 @@ RelationIdGetRelation(Oid relationId)
  * ----------------------------------------------------------------
  */
 
+/* ResourceOwner callbacks to track relcache references */
+static void ResOwnerReleaseRelation(Datum res);
+static void ResOwnerPrintRelCacheLeakWarning(Datum res);
+
+static ResourceOwnerFuncs relref_resowner_funcs =
+{
+	.name = "relcache reference",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseRelation,
+	.PrintLeakWarning = ResOwnerPrintRelCacheLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberRelationRef(owner, rel) \
+	ResourceOwnerRemember(owner, PointerGetDatum(rel), &relref_resowner_funcs)
+#define ResourceOwnerForgetRelationRef(owner, rel) \
+	ResourceOwnerForget(owner, PointerGetDatum(rel), &relref_resowner_funcs)
+
 /*
  * RelationIncrementReferenceCount
  *		Increments relation reference count.
@@ -2126,7 +2145,7 @@ RelationIdGetRelation(Oid relationId)
 void
 RelationIncrementReferenceCount(Relation rel)
 {
-	ResourceOwnerEnlargeRelationRefs(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 	rel->rd_refcnt += 1;
 	if (!IsBootstrapProcessingMode())
 		ResourceOwnerRememberRelationRef(CurrentResourceOwner, rel);
@@ -6776,3 +6795,21 @@ unlink_initfile(const char *initfilename, int elevel)
 							initfilename)));
 	}
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+static void
+ResOwnerPrintRelCacheLeakWarning(Datum res)
+{
+	Relation	rel = (Relation) res;
+
+	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
+		 RelationGetRelationName(rel));
+}
+
+static void
+ResOwnerReleaseRelation(Datum res)
+{
+	RelationClose((Relation) res);
+}
diff --git a/src/backend/utils/resowner/README b/src/backend/utils/resowner/README
index f94c9700df4..e32f7a27384 100644
--- a/src/backend/utils/resowner/README
+++ b/src/backend/utils/resowner/README
@@ -54,12 +54,18 @@ The basic operations on a ResourceOwner are:
 * delete a ResourceOwner (including child owner objects); all resources
   must have been released beforehand
 
-This API directly supports the resource types listed in the definition of
-ResourceOwnerData struct in src/backend/utils/resowner/resowner.c.
-Other objects can be associated with a ResourceOwner by recording the address
-of the owning ResourceOwner in such an object.  There is an API for other
-modules to get control during ResourceOwner release, so that they can scan
-their own data structures to find the objects that need to be deleted.
+ResourceOwner can record ownership of many different kinds of resources.  In
+core PostgreSQL, it is used for buffer pins, lmgr locks, and catalog cache
+references, to name a few examples.  ResourceOwner treats all resources the
+same, and extensions can define new kinds of resources by filling in a
+ResourceOwnerFuncs struct with suitable callback functions.
+
+There is also an API for other modules to get control during ResourceOwner
+release, so that they can scan their own data structures to find the objects
+that need to be deleted.  See RegisterResourceReleaseCallback function.
+This used to be the only way for extensions to use the resource owner
+mechanism with new kinds of objects; nowadays it easier to write custom
+ResourceOwnerFuncs callbacks.
 
 Locks are handled specially because in non-error situations a lock should
 be held until end of transaction, even if it was originally taken by a
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 37b43ee1f8c..3a7f08ea2bf 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -20,76 +20,45 @@
  */
 #include "postgres.h"
 
-#include "common/cryptohash.h"
-#include "common/hmac.h"
 
 /*
+	ResourceOwnerFuncs *kind;	/* NULL indicates a free hash table slot */
+} ResourceElem;
 
 /*
+#define RESOWNER_ARRAY_SIZE 32
 
 /*
+#define RESOWNER_HASH_INIT_SIZE 64
 
 /*
 
 /*
  * To speed up bulk releasing or reassigning locks from a resource owner to
@@ -119,24 +88,33 @@ typedef struct ResourceOwnerData
 	ResourceOwner nextchild;	/* next child of same parent */
 	const char *name;			/* name (just for debugging) */
 
-	/* We have built-in support for remembering: */
-	ResourceArray bufferarr;	/* owned buffers */
-	ResourceArray catrefarr;	/* catcache references */
-	ResourceArray catlistrefarr;	/* catcache-list pins */
-	ResourceArray relrefarr;	/* relcache references */
-	ResourceArray planrefarr;	/* plancache references */
-	ResourceArray tupdescarr;	/* tupdesc references */
-	ResourceArray snapshotarr;	/* snapshot references */
-	ResourceArray filearr;		/* open temporary files */
-	ResourceArray dsmarr;		/* dynamic shmem segments */
-	ResourceArray jitarr;		/* JIT contexts */
-	ResourceArray cryptohasharr;	/* cryptohash contexts */
-	ResourceArray hmacarr;		/* HMAC contexts */
+	/*
+	 * These structs keep track of the objects registered with this owner.
+	 *
+	 * We manage a small set of references by keeping them in a simple array.
+	 * When the array gets full, all the elements in the array are moved to a
+	 * hash table.  This way, the array always contains a few most recently
+	 * remembered references.  To find a particular reference, you need to
+	 * search both the array and the hash table.
+	 */
+	ResourceElem arr[RESOWNER_ARRAY_SIZE];
+	uint32		narr;			/* how many items are stored in the array */
+
+	/*
+	 * The hash table.  Uses open-addressing.  'nhash' is the number of items
+	 * present; if it would exceed 'grow_at', we enlarge it and re-hash.
+	 * 'grow_at' should be rather less than 'capacity' so that we don't waste
+	 * too much time searching for empty slots.
+	 */
+	ResourceElem *hash;
+	uint32		nhash;			/* how many items are stored in the hash */
+	uint32		capacity;		/* allocated length of hash[] */
+	uint32		grow_at;		/* grow hash when reach this */

 /*****************************************************************************
@@ -148,6 +126,18 @@ ResourceOwner CurTransactionResourceOwner = NULL;
 ResourceOwner TopTransactionResourceOwner = NULL;
 ResourceOwner AuxProcessResourceOwner = NULL;
 
@@ -162,298 +152,340 @@ static ResourceReleaseCallbackItem *ResourceRelease_callbacks = NULL;

 /* Internal routines */
-static void ResourceArrayInit(ResourceArray *resarr, Datum invalidval);
-static void ResourceArrayEnlarge(ResourceArray *resarr);
-static void ResourceArrayAdd(ResourceArray *resarr, Datum value);
-static bool ResourceArrayRemove(ResourceArray *resarr, Datum value);
-static bool ResourceArrayGetAny(ResourceArray *resarr, Datum *value);
-static void ResourceArrayFree(ResourceArray *resarr);
+static inline uint32 hash_resource_elem(Datum value, ResourceOwnerFuncs *kind);
+static void ResourceOwnerAddToHash(ResourceOwner owner, Datum value,
+								   ResourceOwnerFuncs *kind);
+static void ResourceOwnerReleaseAll(ResourceOwner owner,
+									ResourceReleasePhase phase,
+									bool printLeakWarnings);
 static void ResourceOwnerReleaseInternal(ResourceOwner owner,
 										 ResourceReleasePhase phase,
 										 bool isCommit,
 										 bool isTopLevel);
 static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);
-static void PrintRelCacheLeakWarning(Relation rel);
-static void PrintPlanCacheLeakWarning(CachedPlan *plan);
-static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
-static void PrintSnapshotLeakWarning(Snapshot snapshot);
-static void PrintFileLeakWarning(File file);
-static void PrintDSMLeakWarning(dsm_segment *seg);
-static void PrintCryptoHashLeakWarning(Datum handle);
-static void PrintHMACLeakWarning(Datum handle);

+static inline uint32
+hash_resource_elem(Datum value, ResourceOwnerFuncs *kind)
+{
+	Datum		data[2];
+
+	data[0] = value;
+	data[1] = PointerGetDatum(kind);
+
+	return hash_bytes((unsigned char *) &data, 2 * SIZEOF_DATUM);
+}
 
 /*
+ * Adds 'value' of given 'kind' to the ResourceOwner's hash table
+ResourceOwnerAddToHash(ResourceOwner owner, Datum value, ResourceOwnerFuncs *kind)
+	Assert(kind != NULL);
+	idx = hash_resource_elem(value, kind) & mask;
+			break;				/* found a free slot */
 
 /*
- * Make sure there is room for at least one more resource in an array.
- *
- * This is separate from actually inserting a resource because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
+ * Call the ReleaseResource callback on entries with given 'phase'.
  */
 static void
-ResourceArrayEnlarge(ResourceArray *resarr)
+ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase,
+						bool printLeakWarnings)
 {
-	uint32		i,
-				oldcap,
-				newcap;
-	Datum	   *olditemsarr;
-	Datum	   *newitemsarr;
+	bool		found;
+	int			capacity;
 
-	if (resarr->nitems < resarr->maxitems)
-		return;					/* no work needed */
-
-	olditemsarr = resarr->itemsarr;
-	oldcap = resarr->capacity;
-
-	/* Double the capacity of the array (capacity must stay a power of 2!) */
-	newcap = (oldcap > 0) ? oldcap * 2 : RESARRAY_INIT_SIZE;
-	newitemsarr = (Datum *) MemoryContextAlloc(TopMemoryContext,
-											   newcap * sizeof(Datum));
-	for (i = 0; i < newcap; i++)
-		newitemsarr[i] = resarr->invalidval;
+	/*
+	 * First handle all the entries in the array.
+	 *
+	 * Note that ReleaseResource() will call ResourceOwnerForget) and remove
+	 * the entry from our array, so we just have to iterate till there is
+	 * nothing left to remove.
+	 */
+	do
+	{
+		found = false;
+		for (int i = 0; i < owner->narr; i++)
+		{
+			if (owner->arr[i].kind->phase == phase)
+			{
+				Datum		value = owner->arr[i].item;
+				ResourceOwnerFuncs *kind = owner->arr[i].kind;

-	if (olditemsarr != NULL)
-	{
 		/*
-		 * Transfer any pre-existing entries into the new array; they don't
-		 * necessarily go where they were before, so this simple logic is the
-		 * best way.  Note that if we were managing the set as a simple array,
-		 * the entries after nitems are garbage, but that shouldn't matter
-		 * because we won't get here unless nitems was equal to oldcap.
+		 * If any resources were released, check again because some of the
+		 * elements might have been moved by the callbacks.  We don't want to
+		 * miss them.
 		 */
-		for (i = 0; i < oldcap; i++)
+	} while (found && owner->narr > 0);
+
+	/*
+	 * Ok, the array has now been handled.  Then the hash.  Like with the
+	 * array, ReleaseResource() will remove the entry from the hash.
+	 */
+	do
+	{
+		capacity = owner->capacity;
+		for (int idx = 0; idx < capacity; idx++)
 		{
-			if (olditemsarr[i] != resarr->invalidval)
-				ResourceArrayAdd(resarr, olditemsarr[i]);
+			while (owner->hash[idx].kind != NULL &&
+				   owner->hash[idx].kind->phase == phase)
+			{
+				Datum		value = owner->hash[idx].item;
+				ResourceOwnerFuncs *kind = owner->hash[idx].kind;
+
+				if (printLeakWarnings)
+					kind->PrintLeakWarning(value);
+				kind->ReleaseResource(value);
+
+				/*
+				 * If the same resource is remembered more than once in this
+				 * resource owner, the ReleaseResource callback might've
+				 * released a different copy of it.  Because of that, loop to
+				 * check the same index again.
+				 */
+			}
 		}
 
-		/* And release old array. */
-		pfree(olditemsarr);
-	}
-
-	Assert(resarr->nitems < resarr->maxitems);
+		/*
+		 * It's possible that the callbacks acquired more resources, causing
+		 * the hash table to grow and the existing entries to be moved around.
+		 * If that happened, scan the hash table again, so that we don't miss
+		 * entries that were moved.  (XXX: I'm not sure if any of the
+		 * callbacks actually do that, but this is cheap to check, and better
+		 * safe than sorry.)
+		 */
+		Assert(owner->capacity >= capacity);
+	} while (capacity != owner->capacity);
 }
 
+
+/*****************************************************************************
+ *	  EXPORTED ROUTINES														 *
+ *****************************************************************************/
+
+
 /*
- * Add a resource to ResourceArray
+ * ResourceOwnerCreate
+ *		Create an empty ResourceOwner.
  *
- * Caller must have previously done ResourceArrayEnlarge()
+ * All ResourceOwner objects are kept in TopMemoryContext, since they should
+ * only be freed explicitly.
  */
-static void
-ResourceArrayAdd(ResourceArray *resarr, Datum value)
+ResourceOwner
+ResourceOwnerCreate(ResourceOwner parent, const char *name)
 {
-	uint32		idx;
+	ResourceOwner owner;
 
-	Assert(value != resarr->invalidval);
-	Assert(resarr->nitems < resarr->maxitems);
+	owner = (ResourceOwner) MemoryContextAllocZero(TopMemoryContext,
+												   sizeof(ResourceOwnerData));
+	owner->name = name;
 
-	if (RESARRAY_IS_ARRAY(resarr))
+	if (parent)
 	{
-		/* Append to linear array. */
-		idx = resarr->nitems;
+		owner->parent = parent;
+		owner->nextchild = parent->firstchild;
+		parent->firstchild = owner;
 	}
-	else
-	{
-		/* Insert into first free slot at or after hash location. */
-		uint32		mask = resarr->capacity - 1;
 
+	return owner;
 }
 
 /*
- * Remove a resource from ResourceArray
+ * Make sure there is room for at least one more resource in an array.
  *
- * Returns true on success, false if resource was not found.
+ * This is separate from actually inserting a resource because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
  *
- * Note: if same resource ID appears more than once, one instance is removed.
+ * NB: Make sure there are no unrelated ResourceOwnerRemember() calls between
+ * your ResourceOwnerEnlarge() call and the ResourceOwnerRemember() call that
+ * you reserved the space for!
  */
-static bool
-ResourceArrayRemove(ResourceArray *resarr, Datum value)
+void
+ResourceOwnerEnlarge(ResourceOwner owner)
 {
-	uint32		i,
-				idx,
-				lastidx = resarr->lastidx;
-
-	Assert(value != resarr->invalidval);
+	if (owner->narr < RESOWNER_ARRAY_SIZE)
+		return;					/* no work needed */
 
-	/* Search through all items, but try lastidx first. */
-	if (RESARRAY_IS_ARRAY(resarr))
+	/* Is there space in the hash? If not, enlarge it. */
+	if (owner->narr + owner->nhash >= owner->grow_at)
 	{
-		if (lastidx < resarr->nitems &&
-			resarr->itemsarr[lastidx] == value)
-		{
-			resarr->itemsarr[lastidx] = resarr->itemsarr[resarr->nitems - 1];
-			resarr->nitems--;
-			/* Update lastidx to make reverse-order removals fast. */
-			resarr->lastidx = resarr->nitems - 1;
-			return true;
-		}
-		for (i = 0; i < resarr->nitems; i++)
+		uint32		i,
+					oldcap,
+					newcap;
+		ResourceElem *oldhash;
+		ResourceElem *newhash;
+
+		oldhash = owner->hash;
+		oldcap = owner->capacity;
+
+		/* Double the capacity (it must stay a power of 2!) */
+		newcap = (oldcap > 0) ? oldcap * 2 : RESOWNER_HASH_INIT_SIZE;
+		newhash = (ResourceElem *) MemoryContextAllocZero(TopMemoryContext,
+														  newcap * sizeof(ResourceElem));
+
+		/*
+		 * We assume we can't fail below this point, so OK to scribble on the
+		 * owner
+		 */
+		owner->hash = newhash;
+		owner->capacity = newcap;
+		owner->grow_at = RESOWNER_HASH_MAX_ITEMS(newcap);
+		owner->nhash = 0;
+
+		if (oldhash != NULL)
 		{
-			if (resarr->itemsarr[i] == value)
+			/*
+			 * Transfer any pre-existing entries into the new hash table; they
+			 * don't necessarily go where they were before, so this simple
+			 * logic is the best way.
+			 */
+			for (i = 0; i < oldcap; i++)
 			{
-				resarr->itemsarr[i] = resarr->itemsarr[resarr->nitems - 1];
-				resarr->nitems--;
-				/* Update lastidx to make reverse-order removals fast. */
-				resarr->lastidx = resarr->nitems - 1;
-				return true;
+				if (oldhash[i].kind != NULL)
+					ResourceOwnerAddToHash(owner, oldhash[i].item, oldhash[i].kind);
 			}
+
+			/* And release old hash table. */
+			pfree(oldhash);
 		}
 	}
-	else
-	{
-		uint32		mask = resarr->capacity - 1;
 
+	Assert(owner->narr == RESOWNER_ARRAY_SIZE);
+		ResourceOwnerAddToHash(owner, owner->arr[i].item, owner->arr[i].kind);
 	}
+	owner->narr = 0;
 
-	return false;
 
 /*

-		for (;;)
-		{
-			resarr->lastidx &= mask;
-			if (resarr->itemsarr[resarr->lastidx] != resarr->invalidval)
-				break;
-			resarr->lastidx++;
-		}
+	if (owner->narr >= RESOWNER_ARRAY_SIZE)
+	{
+		/* forgot to call ResourceOwnerEnlarge? */
+		elog(ERROR, "ResourceOwnerRemember called but array was full");
 	}

 /*
 
-/*****************************************************************************
- *	  EXPORTED ROUTINES														 *
- *****************************************************************************/
+#ifdef RESOWNER_TRACE
+	elog(LOG, "FORGET %d: owner %p value " UINT64_FORMAT ", kind: %s",
+		 resowner_trace_counter++, owner, DatumGetUInt64(value), kind->name);
+#endif

-/*
- * ResourceOwnerCreate
- *		Create an empty ResourceOwner.
- *
- * All ResourceOwner objects are kept in TopMemoryContext, since they should
- * only be freed explicitly.
- */
-ResourceOwner
-ResourceOwnerCreate(ResourceOwner parent, const char *name)
-{
-	ResourceOwner owner;
+#ifdef RESOWNER_STATS
+			narray_lookups++;
+#endif
 
-	owner = (ResourceOwner) MemoryContextAllocZero(TopMemoryContext,
-												   sizeof(ResourceOwnerData));
-	owner->name = name;
+			return;
+		}
+	}
 
-	if (parent)
+	/* Search hash */
+	if (owner->nhash > 0)
 	{
-		owner->parent = parent;
-		owner->nextchild = parent->firstchild;
-		parent->firstchild = owner;
-	}
+		uint32		mask = owner->capacity - 1;
 
-	ResourceArrayInit(&(owner->cryptohasharr), PointerGetDatum(NULL));
-	ResourceArrayInit(&(owner->hmacarr), PointerGetDatum(NULL));
+		idx = hash_resource_elem(value, kind) & mask;
 
-	return owner;
+	/*
+	 * Use %p to print the reference, since most objects tracked by a resource
+	 * owner are pointers.  It's a bit misleading if it's not a pointer, but
+	 * this is a programmer error, anyway.
+	 */
+	elog(ERROR, "%s %p is not owned by resource owner %s",
+		 kind->name, DatumGetPointer(value), owner->name);
 }
 
 /*
@@ -490,6 +522,15 @@ ResourceOwnerRelease(ResourceOwner owner,
 {
 	/* There's not currently any setup needed before recursing */
 	ResourceOwnerReleaseInternal(owner, phase, isCommit, isTopLevel);
+
+#ifdef RESOWNER_STATS
+	if (isTopLevel)
+	{
+		elog(LOG, "RESOWNER STATS: lookups: array %d, hash %d", narray_lookups, nhash_lookups);
+		narray_lookups = 0;
+		nhash_lookups = 0;
+	}
+#endif
 }
 
 static void
@@ -502,7 +543,6 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	ResourceOwner save;
 	ResourceReleaseCallbackItem *item;
 	ResourceReleaseCallbackItem *next;
-	Datum		foundres;
 
 	/* Recurse to handle descendants */
 	for (child = owner->firstchild; child != NULL; child = child->nextchild)
@@ -518,71 +558,13 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
 	{
 		/*
-		 * Release buffer pins.  Note that ReleaseBuffer will remove the
-		 * buffer entry from our array, so we just have to iterate till there
-		 * are none.
+		 * Release all resources that need to be released before the locks.
 		 *
-		 * During a commit, there shouldn't be any remaining pins --- that
-		 * would indicate failure to clean up the executor correctly --- so
-		 * issue warnings.  In the abort case, just clean up quietly.
+		 * During a commit, there shouldn't be any remaining resources ---
+		 * that would indicate failure to clean up the executor correctly ---
+		 * so issue warnings.  In the abort case, just clean up quietly.
 		 */
-		while (ResourceArrayGetAny(&(owner->bufferarr), &foundres))
-		{
-			Buffer		res = DatumGetBuffer(foundres);
-
-			if (isCommit)
-				PrintBufferLeakWarning(res);
-			ReleaseBuffer(res);
-		}
-
-		/* Ditto for relcache references */
-		while (ResourceArrayGetAny(&(owner->relrefarr), &foundres))
-		{
-			Relation	res = (Relation) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintRelCacheLeakWarning(res);
-			RelationClose(res);
-		}
-
-		/* Ditto for dynamic shared memory segments */
-		while (ResourceArrayGetAny(&(owner->dsmarr), &foundres))
-		{
-			dsm_segment *res = (dsm_segment *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintDSMLeakWarning(res);
-			dsm_detach(res);
-		}
-
-		/* Ditto for JIT contexts */
-		while (ResourceArrayGetAny(&(owner->jitarr), &foundres))
-		{
-			JitContext *context = (JitContext *) DatumGetPointer(foundres);
-
-			jit_release_context(context);
-		}
-
-		/* Ditto for cryptohash contexts */
-		while (ResourceArrayGetAny(&(owner->cryptohasharr), &foundres))
-		{
-			pg_cryptohash_ctx *context =
-			(pg_cryptohash_ctx *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintCryptoHashLeakWarning(foundres);
-			pg_cryptohash_free(context);
-		}
-
-		/* Ditto for HMAC contexts */
-		while (ResourceArrayGetAny(&(owner->hmacarr), &foundres))
-		{
-			pg_hmac_ctx *context = (pg_hmac_ctx *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintHMACLeakWarning(foundres);
-			pg_hmac_free(context);
-		}
+		ResourceOwnerReleaseAll(owner, phase, isCommit);
 	}
 	else if (phase == RESOURCE_RELEASE_LOCKS)
 	{
@@ -635,70 +617,9 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 	else if (phase == RESOURCE_RELEASE_AFTER_LOCKS)
 	{
 		/*
-		 * Release catcache references.  Note that ReleaseCatCache will remove
-		 * the catref entry from our array, so we just have to iterate till
-		 * there are none.
-		 *
-		 * As with buffer pins, warn if any are left at commit time.
+		 * Release all resources that need to be released after the locks.
 		 */
-		while (ResourceArrayGetAny(&(owner->catrefarr), &foundres))
-		{
-			HeapTuple	res = (HeapTuple) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintCatCacheLeakWarning(res);
-			ReleaseCatCache(res);
-		}
-
-		/* Ditto for catcache lists */
-		while (ResourceArrayGetAny(&(owner->catlistrefarr), &foundres))
-		{
-			CatCList   *res = (CatCList *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintCatCacheListLeakWarning(res);
-			ReleaseCatCacheList(res);
-		}
-
-		/* Ditto for plancache references */
-		while (ResourceArrayGetAny(&(owner->planrefarr), &foundres))
-		{
-			CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintPlanCacheLeakWarning(res);
-			ReleaseCachedPlan(res, owner);
-		}
-
-		/* Ditto for tupdesc references */
-		while (ResourceArrayGetAny(&(owner->tupdescarr), &foundres))
-		{
-			TupleDesc	res = (TupleDesc) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintTupleDescLeakWarning(res);
-			DecrTupleDescRefCount(res);
-		}
-
-		/* Ditto for snapshot references */
-		while (ResourceArrayGetAny(&(owner->snapshotarr), &foundres))
-		{
-			Snapshot	res = (Snapshot) DatumGetPointer(foundres);
-
-			if (isCommit)
-				PrintSnapshotLeakWarning(res);
-			UnregisterSnapshot(res);
-		}
-
-		/* Ditto for temporary files */
-		while (ResourceArrayGetAny(&(owner->filearr), &foundres))
-		{
-			File		res = DatumGetFile(foundres);
-
-			if (isCommit)
-				PrintFileLeakWarning(res);
-			FileClose(res);
-		}
+		ResourceOwnerReleaseAll(owner, phase, isCommit);
 	}
 
 	/* Let add-on modules get a chance too */
@@ -722,13 +643,42 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 void
 ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner)
 {
-	Datum		foundres;
+	/* array first */
+	for (int i = 0; i < owner->narr; i++)
+	{
+		if (owner->arr[i].kind == &planref_resowner_funcs)
+		{
+			CachedPlan *planref = (CachedPlan *) DatumGetPointer(owner->arr[i].item);
+
+			owner->arr[i] = owner->arr[owner->narr - 1];
+			owner->narr--;
+			i--;
+
+			/*
+			 * pass 'NULL' because we already removed the entry from the
+			 * resowner
+			 */
+			ReleaseCachedPlan(planref, NULL);
+		}
+	}

-		ReleaseCachedPlan(res, owner);
+			/*
+			 * pass 'NULL' because we already removed the entry from the
+			 * resowner
+			 */
+			ReleaseCachedPlan(planref, NULL);
+		}
 	}
 }
 
@@ -745,20 +695,15 @@ ResourceOwnerDelete(ResourceOwner owner)
 	Assert(owner != CurrentResourceOwner);
 
-	Assert(owner->cryptohasharr.nitems == 0);
-	Assert(owner->hmacarr.nitems == 0);
 
+#ifdef RESOWNER_TRACE
@@ -774,19 +719,8 @@ ResourceOwnerDelete(ResourceOwner owner)
 
-	ResourceArrayFree(&(owner->cryptohasharr));
-	ResourceArrayFree(&(owner->hmacarr));
-
+	if (owner->hash)
+		pfree(owner->hash);
 	pfree(owner);
 }
 
@@ -844,11 +778,10 @@ ResourceOwnerNewParent(ResourceOwner owner,
 /*
  * Register or deregister callback functions for resource cleanup
  *
- * These functions are intended for use by dynamically loaded modules.
- * For built-in modules we generally just hardwire the appropriate calls.
- *
- * Note that the callback occurs post-commit or post-abort, so the callback
- * functions can only do noncritical cleanup.
+ * These functions can be used by dynamically loaded modules.  These used
+ * to be the only way for an extension to register custom resource types
+ * with a resource owner, but nowadays it is easier to define a new
+ * ResourceOwnerFuncs instance with custom callbacks.
  */
 void
 RegisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg)
@@ -938,58 +871,20 @@ ReleaseAuxProcessResourcesCallback(int code, Datum arg)
 	ReleaseAuxProcessResources(isCommit);
 }
 
-
 /*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * buffer array.
+ * Remember that a Local Lock is owned by a ResourceOwner
  *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
+ * This is different from the other Remember functions in that the list of
+ * locks is only a lossy cache.  It can hold up to MAX_RESOWNER_LOCKS entries,
+ * and when it overflows, we stop tracking locks.  The point of only remembering
+ * only up to MAX_RESOWNER_LOCKS entries is that if a lot of locks are held,
+ * ResourceOwnerForgetLock doesn't need to scan through a large array to find
+ * the entry.
  */
 void
-ResourceOwnerEnlargeBuffers(ResourceOwner owner)
+ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock)
 {
-	/* We used to allow pinning buffers without a resowner, but no more */
-	Assert(owner != NULL);
-	ResourceArrayEnlarge(&(owner->bufferarr));
-}
-
-/*
- * Remember that a buffer pin is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeBuffers()
- */
-void
-ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer)
-{
-	ResourceArrayAdd(&(owner->bufferarr), BufferGetDatum(buffer));
-}
-
-/*
- * Forget that a buffer pin is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
-{
-	if (!ResourceArrayRemove(&(owner->bufferarr), BufferGetDatum(buffer)))
-		elog(ERROR, "buffer %d is not owned by resource owner %s",
-			 buffer, owner->name);
-}
-
-/*
- * Remember that a Local Lock is owned by a ResourceOwner
- *
- * This is different from the other Remember functions in that the list of
- * locks is only a lossy cache. It can hold up to MAX_RESOWNER_LOCKS entries,
- * and when it overflows, we stop tracking locks. The point of only remembering
- * only up to MAX_RESOWNER_LOCKS entries is that if a lot of locks are held,
- * ResourceOwnerForgetLock doesn't need to scan through a large array to find
- * the entry.
- */
-void
-ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock)
-{
-	Assert(locallock != NULL);
+	Assert(locallock != NULL);
 
 	if (owner->nlocks > MAX_RESOWNER_LOCKS)
 		return;					/* we have already overflowed */
@@ -1027,469 +922,3 @@ ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock)
 	elog(ERROR, "lock reference %p is not owned by resource owner %s",
 		 locallock, owner->name);
 }
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * catcache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeCatCacheRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->catrefarr));
-}
-
-/*
- * Remember that a catcache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeCatCacheRefs()
- */
-void
-ResourceOwnerRememberCatCacheRef(ResourceOwner owner, HeapTuple tuple)
-{
-	ResourceArrayAdd(&(owner->catrefarr), PointerGetDatum(tuple));
-}
-
-/*
- * Forget that a catcache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetCatCacheRef(ResourceOwner owner, HeapTuple tuple)
-{
-	if (!ResourceArrayRemove(&(owner->catrefarr), PointerGetDatum(tuple)))
-		elog(ERROR, "catcache reference %p is not owned by resource owner %s",
-			 tuple, owner->name);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * catcache-list reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeCatCacheListRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->catlistrefarr));
-}
-
-/*
- * Remember that a catcache-list reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeCatCacheListRefs()
- */
-void
-ResourceOwnerRememberCatCacheListRef(ResourceOwner owner, CatCList *list)
-{
-	ResourceArrayAdd(&(owner->catlistrefarr), PointerGetDatum(list));
-}
-
-/*
- * Forget that a catcache-list reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetCatCacheListRef(ResourceOwner owner, CatCList *list)
-{
-	if (!ResourceArrayRemove(&(owner->catlistrefarr), PointerGetDatum(list)))
-		elog(ERROR, "catcache list reference %p is not owned by resource owner %s",
-			 list, owner->name);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * relcache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeRelationRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->relrefarr));
-}
-
-/*
- * Remember that a relcache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeRelationRefs()
- */
-void
-ResourceOwnerRememberRelationRef(ResourceOwner owner, Relation rel)
-{
-	ResourceArrayAdd(&(owner->relrefarr), PointerGetDatum(rel));
-}
-
-/*
- * Forget that a relcache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetRelationRef(ResourceOwner owner, Relation rel)
-{
-	if (!ResourceArrayRemove(&(owner->relrefarr), PointerGetDatum(rel)))
-		elog(ERROR, "relcache reference %s is not owned by resource owner %s",
-			 RelationGetRelationName(rel), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintRelCacheLeakWarning(Relation rel)
-{
-	elog(WARNING, "relcache reference leak: relation \"%s\" not closed",
-		 RelationGetRelationName(rel));
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * plancache reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargePlanCacheRefs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->planrefarr));
-}
-
-/*
- * Remember that a plancache reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargePlanCacheRefs()
- */
-void
-ResourceOwnerRememberPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
-{
-	ResourceArrayAdd(&(owner->planrefarr), PointerGetDatum(plan));
-}
-
-/*
- * Forget that a plancache reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetPlanCacheRef(ResourceOwner owner, CachedPlan *plan)
-{
-	if (!ResourceArrayRemove(&(owner->planrefarr), PointerGetDatum(plan)))
-		elog(ERROR, "plancache reference %p is not owned by resource owner %s",
-			 plan, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintPlanCacheLeakWarning(CachedPlan *plan)
-{
-	elog(WARNING, "plancache reference leak: plan %p not closed", plan);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * tupdesc reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeTupleDescs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->tupdescarr));
-}
-
-/*
- * Remember that a tupdesc reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeTupleDescs()
- */
-void
-ResourceOwnerRememberTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
-{
-	ResourceArrayAdd(&(owner->tupdescarr), PointerGetDatum(tupdesc));
-}
-
-/*
- * Forget that a tupdesc reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetTupleDesc(ResourceOwner owner, TupleDesc tupdesc)
-{
-	if (!ResourceArrayRemove(&(owner->tupdescarr), PointerGetDatum(tupdesc)))
-		elog(ERROR, "tupdesc reference %p is not owned by resource owner %s",
-			 tupdesc, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintTupleDescLeakWarning(TupleDesc tupdesc)
-{
-	elog(WARNING,
-		 "TupleDesc reference leak: TupleDesc %p (%u,%d) still referenced",
-		 tupdesc, tupdesc->tdtypeid, tupdesc->tdtypmod);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * snapshot reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeSnapshots(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->snapshotarr));
-}
-
-/*
- * Remember that a snapshot reference is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeSnapshots()
- */
-void
-ResourceOwnerRememberSnapshot(ResourceOwner owner, Snapshot snapshot)
-{
-	ResourceArrayAdd(&(owner->snapshotarr), PointerGetDatum(snapshot));
-}
-
-/*
- * Forget that a snapshot reference is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetSnapshot(ResourceOwner owner, Snapshot snapshot)
-{
-	if (!ResourceArrayRemove(&(owner->snapshotarr), PointerGetDatum(snapshot)))
-		elog(ERROR, "snapshot reference %p is not owned by resource owner %s",
-			 snapshot, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintSnapshotLeakWarning(Snapshot snapshot)
-{
-	elog(WARNING, "Snapshot reference leak: Snapshot %p still referenced",
-		 snapshot);
-}
-
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * files reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeFiles(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->filearr));
-}
-
-/*
- * Remember that a temporary file is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeFiles()
- */
-void
-ResourceOwnerRememberFile(ResourceOwner owner, File file)
-{
-	ResourceArrayAdd(&(owner->filearr), FileGetDatum(file));
-}
-
-/*
- * Forget that a temporary file is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetFile(ResourceOwner owner, File file)
-{
-	if (!ResourceArrayRemove(&(owner->filearr), FileGetDatum(file)))
-		elog(ERROR, "temporary file %d is not owned by resource owner %s",
-			 file, owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintFileLeakWarning(File file)
-{
-	elog(WARNING, "temporary file leak: File %d still referenced",
-		 file);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * dynamic shmem segment reference array.
- *
- * This is separate from actually inserting an entry because if we run out
- * of memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeDSMs(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->dsmarr));
-}
-
-/*
- * Remember that a dynamic shmem segment is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeDSMs()
- */
-void
-ResourceOwnerRememberDSM(ResourceOwner owner, dsm_segment *seg)
-{
-	ResourceArrayAdd(&(owner->dsmarr), PointerGetDatum(seg));
-}
-
-/*
- * Forget that a dynamic shmem segment is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetDSM(ResourceOwner owner, dsm_segment *seg)
-{
-	if (!ResourceArrayRemove(&(owner->dsmarr), PointerGetDatum(seg)))
-		elog(ERROR, "dynamic shared memory segment %u is not owned by resource owner %s",
-			 dsm_segment_handle(seg), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintDSMLeakWarning(dsm_segment *seg)
-{
-	elog(WARNING, "dynamic shared memory leak: segment %u still referenced",
-		 dsm_segment_handle(seg));
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * JIT context reference array.
- *
- * This is separate from actually inserting an entry because if we run out of
- * memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeJIT(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->jitarr));
-}
-
-/*
- * Remember that a JIT context is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeJIT()
- */
-void
-ResourceOwnerRememberJIT(ResourceOwner owner, Datum handle)
-{
-	ResourceArrayAdd(&(owner->jitarr), handle);
-}
-
-/*
- * Forget that a JIT context is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle)
-{
-	if (!ResourceArrayRemove(&(owner->jitarr), handle))
-		elog(ERROR, "JIT context %p is not owned by resource owner %s",
-			 DatumGetPointer(handle), owner->name);
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * cryptohash context reference array.
- *
- * This is separate from actually inserting an entry because if we run out of
- * memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeCryptoHash(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->cryptohasharr));
-}
-
-/*
- * Remember that a cryptohash context is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeCryptoHash()
- */
-void
-ResourceOwnerRememberCryptoHash(ResourceOwner owner, Datum handle)
-{
-	ResourceArrayAdd(&(owner->cryptohasharr), handle);
-}
-
-/*
- * Forget that a cryptohash context is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetCryptoHash(ResourceOwner owner, Datum handle)
-{
-	if (!ResourceArrayRemove(&(owner->cryptohasharr), handle))
-		elog(ERROR, "cryptohash context %p is not owned by resource owner %s",
-			 DatumGetPointer(handle), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintCryptoHashLeakWarning(Datum handle)
-{
-	elog(WARNING, "cryptohash context reference leak: context %p still referenced",
-		 DatumGetPointer(handle));
-}
-
-/*
- * Make sure there is room for at least one more entry in a ResourceOwner's
- * hmac context reference array.
- *
- * This is separate from actually inserting an entry because if we run out of
- * memory, it's critical to do so *before* acquiring the resource.
- */
-void
-ResourceOwnerEnlargeHMAC(ResourceOwner owner)
-{
-	ResourceArrayEnlarge(&(owner->hmacarr));
-}
-
-/*
- * Remember that a HMAC context is owned by a ResourceOwner
- *
- * Caller must have previously done ResourceOwnerEnlargeHMAC()
- */
-void
-ResourceOwnerRememberHMAC(ResourceOwner owner, Datum handle)
-{
-	ResourceArrayAdd(&(owner->hmacarr), handle);
-}
-
-/*
- * Forget that a HMAC context is owned by a ResourceOwner
- */
-void
-ResourceOwnerForgetHMAC(ResourceOwner owner, Datum handle)
-{
-	if (!ResourceArrayRemove(&(owner->hmacarr), handle))
-		elog(ERROR, "HMAC context %p is not owned by resource owner %s",
-			 DatumGetPointer(handle), owner->name);
-}
-
-/*
- * Debugging subroutine
- */
-static void
-PrintHMACLeakWarning(Datum handle)
-{
-	elog(WARNING, "HMAC context reference leak: context %p still referenced",
-		 DatumGetPointer(handle));
-}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index f1f2ddac17c..ceff63a3ecf 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -57,6 +57,7 @@
 #include "lib/pairingheap.h"
 #include "miscadmin.h"
 #include "port/pg_lfind.h"
+#include "storage/fd.h"
 #include "storage/predicate.h"
 #include "storage/proc.h"
 #include "storage/procarray.h"
@@ -67,7 +68,7 @@
 #include "utils/memutils.h"
 #include "utils/old_snapshot.h"
 #include "utils/rel.h"
-#include "utils/resowner_private.h"
+#include "utils/resowner.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/timestamp.h"
@@ -175,6 +176,24 @@ static Snapshot CopySnapshot(Snapshot snapshot);
 static void FreeSnapshot(Snapshot snapshot);
 static void SnapshotResetXmin(void);
 
+/* ResourceOwner callbacks to track snapshot references */
+static void ResOwnerReleaseSnapshot(Datum res);
+static void ResOwnerPrintSnapshotLeakWarning(Datum res);
+
+static ResourceOwnerFuncs snapshot_resowner_funcs =
+{
+	.name = "snapshot reference",
+	.phase = RESOURCE_RELEASE_AFTER_LOCKS,
+	.ReleaseResource = ResOwnerReleaseSnapshot,
+	.PrintLeakWarning = ResOwnerPrintSnapshotLeakWarning
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberSnapshot(owner, snap) \
+	ResourceOwnerRemember(owner, PointerGetDatum(snap), &snapshot_resowner_funcs)
+#define ResourceOwnerForgetSnapshot(owner, snap) \
+	ResourceOwnerForget(owner, PointerGetDatum(snap), &snapshot_resowner_funcs)
+
 /*
  * Snapshot fields to be serialized.
  *
@@ -850,7 +869,7 @@ RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
 	snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
 
 	/* and tell resowner.c about it */
-	ResourceOwnerEnlargeSnapshots(owner);
+	ResourceOwnerEnlarge(owner);
 	snap->regd_count++;
 	ResourceOwnerRememberSnapshot(owner, snap);
 
@@ -2379,3 +2398,19 @@ XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot)
 
 	return false;
 }
+
+/*
index 2fb29b1f855..5ed746e7d69 100644
@@ -31,7 +31,6 @@
 #ifndef FRONTEND
 
 /*
@@ -74,6 +73,27 @@ struct pg_cryptohash_ctx
 #endif
 };
 
+/* ResourceOwner callbacks to hold cryptohash contexts */
+#ifndef FRONTEND
+static void ResOwnerReleaseCryptoHash(Datum res);
+static void ResOwnerPrintCryptoHashLeakWarning(Datum res);
+
+static ResourceOwnerFuncs cryptohash_resowner_funcs =
+{
+	/* relcache references */
+	.name = "OpenSSL cryptohash context",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseCryptoHash,
+	.PrintLeakWarning = ResOwnerPrintCryptoHashLeakWarning,
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberCryptoHash(owner, ctx) \
+	ResourceOwnerRemember(owner, PointerGetDatum(ctx), &cryptohash_resowner_funcs)
+#define ResourceOwnerForgetCryptoHash(owner, ctx) \
+	ResourceOwnerForget(owner, PointerGetDatum(ctx), &cryptohash_resowner_funcs)
+#endif
+
 static const char *
 SSLerrmessage(unsigned long ecode)
 {
@@ -104,7 +124,7 @@ pg_cryptohash_create(pg_cryptohash_type type)
 	 * allocation to avoid leaking.
 	 */
 #ifndef FRONTEND
-	ResourceOwnerEnlargeCryptoHash(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 #endif
 
 	ctx = ALLOC(sizeof(pg_cryptohash_ctx));
@@ -138,8 +158,7 @@ pg_cryptohash_create(pg_cryptohash_type type)
 
 #ifndef FRONTEND
 	ctx->resowner = CurrentResourceOwner;
-	ResourceOwnerRememberCryptoHash(CurrentResourceOwner,
-									PointerGetDatum(ctx));
+	ResourceOwnerRememberCryptoHash(CurrentResourceOwner, ctx);
 #endif
 
 	return ctx;
@@ -307,8 +326,7 @@ pg_cryptohash_free(pg_cryptohash_ctx *ctx)
 	EVP_MD_CTX_destroy(ctx->evpctx);
 
 #ifndef FRONTEND
-	ResourceOwnerForgetCryptoHash(ctx->resowner,
-								  PointerGetDatum(ctx));
+	ResourceOwnerForgetCryptoHash(ctx->resowner, ctx);
 #endif
 
 	explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
@@ -351,3 +369,21 @@ pg_cryptohash_error(pg_cryptohash_ctx *ctx)
 	Assert(false);				/* cannot be reached */
 	return _("success");
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+#ifndef FRONTEND
+static void
+ResOwnerReleaseCryptoHash(Datum res)
+{
+	pg_cryptohash_free((pg_cryptohash_ctx *) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintCryptoHashLeakWarning(Datum res)
+{
+	elog(WARNING, "cryptohash context reference leak: context %p still referenced",
+		 DatumGetPointer(res));
+}
+#endif
diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c
index 8874d6a240c..dfe617327ed 100644
--- a/src/common/hmac_openssl.c
+++ b/src/common/hmac_openssl.c
@@ -31,7 +31,6 @@
 #ifndef FRONTEND
 #include "utils/memutils.h"
 #include "utils/resowner.h"
-#include "utils/resowner_private.h"
 #endif
 
 /*
@@ -73,6 +72,27 @@ struct pg_hmac_ctx
 #endif
 };
 
+/* ResourceOwner callbacks to hold HMAC contexts */
+#ifndef FRONTEND
+static void ResOwnerReleaseHMAC(Datum res);
+static void ResOwnerPrintHMACLeakWarning(Datum res);
+
+static ResourceOwnerFuncs hmac_resowner_funcs =
+{
+	/* relcache references */
+	.name = "OpenSSL HMAC context",
+	.phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+	.ReleaseResource = ResOwnerReleaseHMAC,
+	.PrintLeakWarning = ResOwnerPrintHMACLeakWarning,
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberHMAC(owner, ctx) \
+	ResourceOwnerRemember(owner, PointerGetDatum(ctx), &hmac_resowner_funcs)
+#define ResourceOwnerForgetHMAC(owner, ctx) \
+	ResourceOwnerForget(owner, PointerGetDatum(ctx), &hmac_resowner_funcs)
+#endif
+
 static const char *
 SSLerrmessage(unsigned long ecode)
 {
@@ -115,7 +135,7 @@ pg_hmac_create(pg_cryptohash_type type)
 	ERR_clear_error();
 #ifdef HAVE_HMAC_CTX_NEW
 #ifndef FRONTEND
-	ResourceOwnerEnlargeHMAC(CurrentResourceOwner);
+	ResourceOwnerEnlarge(CurrentResourceOwner);
 #endif
 	ctx->hmacctx = HMAC_CTX_new();
 #else
@@ -137,7 +157,7 @@ pg_hmac_create(pg_cryptohash_type type)
 #ifdef HAVE_HMAC_CTX_NEW
 #ifndef FRONTEND
 	ctx->resowner = CurrentResourceOwner;
-	ResourceOwnerRememberHMAC(CurrentResourceOwner, PointerGetDatum(ctx));
+	ResourceOwnerRememberHMAC(CurrentResourceOwner, ctx);
 #endif
 #else
 	memset(ctx->hmacctx, 0, sizeof(HMAC_CTX));
@@ -303,7 +323,7 @@ pg_hmac_free(pg_hmac_ctx *ctx)
 #ifdef HAVE_HMAC_CTX_FREE
 	HMAC_CTX_free(ctx->hmacctx);
 #ifndef FRONTEND
-	ResourceOwnerForgetHMAC(ctx->resowner, PointerGetDatum(ctx));
+	ResourceOwnerForgetHMAC(ctx->resowner, ctx);
 #endif
 #else
 	explicit_bzero(ctx->hmacctx, sizeof(HMAC_CTX));
@@ -346,3 +366,21 @@ pg_hmac_error(pg_hmac_ctx *ctx)
 	Assert(false);				/* cannot be reached */
 	return _("success");
 }
+
+/*
+ * ResourceOwner callbacks
+ */
+#ifndef FRONTEND
+static void
+ResOwnerReleaseHMAC(Datum res)
+{
+	pg_hmac_free((pg_hmac_ctx *) DatumGetPointer(res));
+}
+
+static void
+ResOwnerPrintHMACLeakWarning(Datum res)
+{
+	elog(WARNING, "HMAC context reference leak: context %p still referenced",
+		 DatumGetPointer(res));
+}
+#endif
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 406db6be783..a4ca7c9ee6e 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -382,6 +382,15 @@ typedef struct CkptSortItem
 
 extern PGDLLIMPORT CkptSortItem *CkptBufferIds;
 
+/* ResourceOwner callbacks to hold buffer pins */
+extern ResourceOwnerFuncs buffer_resowner_funcs;
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+#define ResourceOwnerRememberBuffer(owner, buffer) \
+	ResourceOwnerRemember(owner, Int32GetDatum(buffer), &buffer_resowner_funcs)
+#define ResourceOwnerForgetBuffer(owner, buffer) \
+	ResourceOwnerForget(owner, Int32GetDatum(buffer), &buffer_resowner_funcs)
+
 /*
  * Internal buffer management routines
  */
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index d81e6fabb73..4b945576655 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -225,7 +225,4 @@ extern void PrepareToInvalidateCacheTuple(Relation relation,
 										  HeapTuple newtuple,
 										  void (*function) (int, uint32, Oid));
 
index 0499635f594..5e4e1526904 100644
 
+extern ResourceOwnerFuncs planref_resowner_funcs;
+
 #endif							/* PLANCACHE_H */
diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h
index 4aff7015f92..a3612b848c2 100644
--- a/src/include/utils/resowner.h
+++ b/src/include/utils/resowner.h
@@ -50,6 +50,36 @@ typedef enum
 	RESOURCE_RELEASE_AFTER_LOCKS
 } ResourceReleasePhase;
 
+/*
+ * In order to track an object, resowner.c needs a few callbacks for it.
+ * The callbacks for resources of a specific kind are encapsulated in
+ * ResourceOwnerFuncs.
+ *
+ * Note that the callback occurs post-commit or post-abort, so these callback
+ * functions can only do noncritical cleanup.
+ */
+typedef struct ResourceOwnerFuncs
+{
+	const char *name;			/* name for the object kind, for debugging */
+
+	ResourceReleasePhase phase; /* when are these objects released? */
+
+	/*
+	 * Release resource.
+	 *
+	 * NOTE: this must call ResourceOwnerForget to disassociate it with the
+	 * resource owner.
+	 */
+	void		(*ReleaseResource) (Datum res);
+
+	/*
+	 * Print a warning, when a resource has not been properly released before
+	 * commit.
+	 */
+	void		(*PrintLeakWarning) (Datum res);
+
+} ResourceOwnerFuncs;
+
 /*
  *	Dynamically loaded modules can get control during ResourceOwnerRelease
  *	by providing a callback of this form.
@@ -71,16 +101,29 @@ extern void ResourceOwnerRelease(ResourceOwner owner,
 								 ResourceReleasePhase phase,
 								 bool isCommit,
 								 bool isTopLevel);
-extern void ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner);
 extern void ResourceOwnerDelete(ResourceOwner owner);
 extern ResourceOwner ResourceOwnerGetParent(ResourceOwner owner);
 extern void ResourceOwnerNewParent(ResourceOwner owner,
 								   ResourceOwner newparent);
+
+extern void ResourceOwnerEnlarge(ResourceOwner owner);
+extern void ResourceOwnerRemember(ResourceOwner owner, Datum res, ResourceOwnerFuncs *kind);
+extern void ResourceOwnerForget(ResourceOwner owner, Datum res, ResourceOwnerFuncs *kind);
+
 extern void RegisterResourceReleaseCallback(ResourceReleaseCallback callback,
 											void *arg);
 extern void UnregisterResourceReleaseCallback(ResourceReleaseCallback callback,
 											  void *arg);
+
 extern void CreateAuxProcessResourceOwner(void);
 extern void ReleaseAuxProcessResources(bool isCommit);
 
index d01cccc27c1..00000000000
@@ -1,112 +0,0 @@
- * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
-/* support for cryptohash context management */
-extern void ResourceOwnerEnlargeCryptoHash(ResourceOwner owner);
-extern void ResourceOwnerRememberCryptoHash(ResourceOwner owner,
-extern void ResourceOwnerForgetCryptoHash(ResourceOwner owner,
-/* support for HMAC context management */
-extern void ResourceOwnerEnlargeHMAC(ResourceOwner owner);
-extern void ResourceOwnerRememberHMAC(ResourceOwner owner,
-extern void ResourceOwnerForgetHMAC(ResourceOwner owner,
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2f02cc8f422..a0ca1f20ac4 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2320,8 +2320,10 @@ ReplicationStateOnDisk
 ResTarget
 ReservoirState
 ReservoirStateData
-ResourceArray
+ResourceElem
 ResourceOwner
+ResourceOwnerData
+ResourceOwnerFuncs
 ResourceReleaseCallback
 ResourceReleaseCallbackItem
 ResourceReleasePhase
-- 
2.30.2

From 12cc7e568ae97ffd6fc57f4024b81d5da480ab4f Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Mon, 31 Oct 2022 10:49:06 +0100
Subject: [PATCH v12 3/3] Use a faster hash function in resource owners.

This buys back some of the performance loss that we otherwise saw from the
previous commit.

Reviewed-by: Aleksander Alekseev, Michael Paquier, Hayato Kuroda, Zhihong Yu
Discussion: https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi
---
 src/backend/utils/resowner/resowner.c | 24 ++++++++++++++++++------
 src/include/common/hashfn.h           | 15 +++++++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 3a7f08ea2bf..c640ab0d227 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -169,15 +169,27 @@ static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);
  *	  INTERNAL ROUTINES														 *
  *****************************************************************************/
 
+/*
+ * Hash function for value+kind combination.
+ */
 static inline uint32
 hash_resource_elem(Datum value, ResourceOwnerFuncs *kind)
 {
-	Datum		data[2];
-
-	data[0] = value;
-	data[1] = PointerGetDatum(kind);
-
-	return hash_bytes((unsigned char *) &data, 2 * SIZEOF_DATUM);
+	/*
+	 * Most resource kinds store a pointer in 'value', and pointers are unique
+	 * all on their own.  But some resources store plain integers (Files and
+	 * Buffers as of this writing), so we want to incorporate the 'kind' in
+	 * the hash too, otherwise those resources will collide a lot.  But
+	 * because there are only a few resource kinds like that - and only a few
+	 * resource kinds to begin with - we don't need to work too hard to mix
+	 * 'kind' into the hash.  Just add it with hash_combine(), it perturbs the
+	 * result enough for our purposes.
+	 */
+#if SIZEOF_DATUM == 8
+	return hash_combine64(murmurhash64((uint64) value), (uint64) kind);
+#else
+	return hash_combine(murmurhash32((uint32) value), (uint32) kind);
+#endif
 }
 
 /*
diff --git a/src/include/common/hashfn.h b/src/include/common/hashfn.h
index 8d539c0a994..c44b4e5f864 100644
--- a/src/include/common/hashfn.h
+++ b/src/include/common/hashfn.h
@@ -101,4 +101,19 @@ murmurhash32(uint32 data)
 	return h;
 }
 
+/* 64-bit variant */
+static inline uint64
+murmurhash64(uint64 data)
+{
+	uint64		h = data;
+
+	h ^= h >> 33;
+	h *= 0xff51afd7ed558ccd;
+	h ^= h >> 33;
+	h *= 0xc4ceb9fe1a85ec53;
+	h ^= h >> 33;
+
+	return h;
+}
+
 #endif							/* HASHFN_H */
-- 
2.30.2

---

From: Aleksander Alekseev <[email-masked]>
Date: Mon, 31 Oct 2022 15:49:37 +0300

Hi Heikki,

> and will commit in the next few days, barring any new objections.

Thanks for resurrecting this patch.

While taking a fresh look at the code I noticed a few things.

In 0002 we have:

```
+    .name = "buffer"
...
+    .name = "File",
```

Not sure why "File" starts with an uppercase letter while "buffer"
starts with a lowercase one. This is naturally not a big deal but
could be worth changing for consistency.

In 0003:

```
+#if SIZEOF_DATUM == 8

Maybe it's worth using PointerGetDatum() + DatumGetInt32() /
DatumGetInt64() inline functions instead of casting Datums and
pointers directly.

These are arguably nitpicks though and shouldn't stop you from merging
the patches as is.

-- 
Best regards,
Aleksander Alekseev

---

From: Aleksander Alekseev <[email-masked]>
Date: Mon, 31 Oct 2022 18:51:41 +0300

Hi hackers,

> Thanks for resurrecting this patch.

Additionally I decided to run `make installcheck-world` on Raspberry
Pi 3. It just terminated successfully.

-- 
Best regards,
Aleksander Alekseev

---

From: Andres Freund <[email-masked]>
Date: Mon, 31 Oct 2022 17:15:23 -0700

Hi,

> the code path between them is, the harder it is to verify that.

This seems to work towards a future where only one kind of resource can be
reserved ahead of time. That doesn't strike me as great.

> extensions to register custom resource kinds.

As a goal I like this.

However, I'm not quite sold on the implementation. Two main worries:

1) As far as I can tell, the way ResourceOwnerReleaseAll() now works seems to
   assume that within a phase the reset order does not matter. I don't think
   that's a good assumption. I e.g. have a patch to replace InProgressBuf with
   resowner handling, and in-progress IO errors should be processed before
   before pins are released.

2) There's quite a few resource types where we actually don't need an entry in
   an array, because we can instead add a dlist_node to the resource -
   avoiding memory overhead and making removal cheap. I have a few pending
   patches that use that approach, and this doesn't really provide a path for
   that anymore.

I did try out the benchmark from
https://postgr.es/m/20221029200025.w7bvlgvamjfo6z44%40awork3.anarazel.de and
the patches performed well, slightly better than my approach of allocating
some initial memory for each resarray.

Greetings,

Andres Freund

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 1 Nov 2022 12:39:39 +0200

> reserved ahead of time. That doesn't strike me as great.

True. It is enough for all the current callers AFAICS, though. I don't 
remember wanting to reserve multiple resources at the same time. 
Usually, the distance between the Enlarge and Remember calls is very 
short. If it's not, it's error-prone anyway, so we should try to keep 
the distance short.

While we're at it, who says that it's enough to reserve space for only 
one resource of a kind either? For example, what if you want to pin two 
buffers?

If we really need to support that, I propose something like this:

/*
  * Reserve a slot in the resource owner.
  *
  * This is separate from actually inserting an entry because if we run out
  * of memory, it's critical to do so *before* acquiring the resource.
  */
ResOwnerReservation *
ResourceOwnerReserve(ResourceOwner owner)

/*
  * Remember that an object is owner by a ResourceOwner
  *
  * 'reservation' is a slot in the resource owner that was pre-reserved
  * by ResOwnerReservation.
  */
void
ResOwnerRemember(ResOwnerReservaton *reservation, Datum value, 
ResourceOwnerFuncs *kind)

>     before pins are released.

Hmm. Currently, you're not supposed to hold any resources at commit. You 
get warnings about resource leaks if a resource owner is not empty on 
ResourceOwnerReleaseAll(). On abort, does the order matter? I'm not 
familiar with your InProgressBuf patch, but I guess you could handle the 
in-progress IO errors in ReleaseBuffer().

If we do need to worry about release order, perhaps add a "priority" or 
"phase" to each resource kind, and release them in priority order. We 
already have before- and after-locks as phases, but we could generalize 
that.

However, I feel that trying to enforce a particular order moves the 
goalposts. If we need that, let's add it as a separate patch later.

>     patches that use that approach, and this doesn't really provide a path for

Is that materially better than using the array? The fast path with an 
array is very fast. If it is better, perhaps we should bite the bullet 
and require a dlist node and use that mechanism for all resource types?

> some initial memory for each resarray.

Thank you, glad to hear that!

- Heikki

---

From: Robert Haas <[email-masked]>
Date: Tue, 1 Nov 2022 08:43:09 -0400

> goalposts. If we need that, let's add it as a separate patch later.

I don't really see it that way, because with the current
implementation, we do all resources of a particular type together,
before moving on to the next type. That seems like a valuable property
to preserve, and I think we should.

---

From: Andres Freund <[email-masked]>
Date: Tue, 1 Nov 2022 08:42:51 -0700

Hi,

On 2022-11-01 12:39:39 +0200, Heikki Linnakangas wrote:
> Hmm. Currently, you're not supposed to hold any resources at commit. You get
> warnings about resource leaks if a resource owner is not empty on
> ResourceOwnerReleaseAll(). On abort, does the order matter? I'm not familiar
> with your InProgressBuf patch, but I guess you could handle the in-progress
> IO errors in ReleaseBuffer().

I was thinking about doing that as well, but it's not really trivial to know
about the in-progress IO at that time, without additional tracking (which
isn't free).

> goalposts. If we need that, let's add it as a separate patch later.

Like Robert, I think that the patch is moving the goalpost...

> Is that materially better than using the array?

It's safe in critical sections. I have a, not really baked but promising,
patch to make WAL writes use AIO. There's no way to know the number of
"asynchronous IOs" needed before entering the critical section.

> The fast path with an array is very fast. If it is better, perhaps we should
> bite the bullet and require a dlist node and use that mechanism for all
> resource types?

I don't think it's suitable for all - you need an exclusively owned region of
memory to embed a list in. That works nicely for some things, but not others
(e.g. buffer pins).

Greetings,

Andres Freund