On 18/10/2025 01:49, Tomas Vondra wrote:
On 10/17/25 12:32, Tomas Vondra wrote:


On 10/17/25 10:31, Heikki Linnakangas wrote:
  typedef struct ResourceElem
  {
      Datum        item;
+    uint32        count;                /* number of occurrences */
      const ResourceOwnerDesc *kind;    /* NULL indicates a free hash
table slot */
  } ResourceElem;

Hmm, the 'count' is not used when the entry is stored in the array.
Perhaps we should have a separate struct for array and hash elements
now. Keeping the array small helps it to fit in CPU caches.

Agreed. I had the same idea yesterday, but I haven't done it yet.

The attached v2 does that - it adds a separate ResourceHashElem for the
has table, and it works. But I'm not sure I like it very much, because
there are two places that relied on both the array and hash table using
the same struct to "walk" it the same way.

For ResourceOwnerSort() it's not too bad, but ResourceOwnerReleaseAll()
now duplicates most of the code. It's not terrible, but also not pretty.
I can't think of a better way, though.

Looks fine to me. The code duplication is not too bad IMO.

Here's a rebased version of the micro-benchmark I used when I was working on the ResourceOwner refactoring (https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi).

I ran it again on my laptop. Different from the one I used back then, so the results are not comparable with the results from that old thread.

Unpatched (commit 18d26140934):

postgres=# \i contrib/resownerbench/snaptest.sql
 numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
       0 |        1 |         11.6 |         11.1
       0 |        5 |         12.1 |         13.1
       0 |       10 |         12.3 |         13.5
       0 |       60 |         14.6 |         19.4
       0 |       70 |         16.0 |         18.1
       0 |      100 |         16.7 |         18.0
       0 |     1000 |         18.1 |         20.7
       0 |    10000 |         21.9 |         29.5
       9 |       10 |         11.0 |         11.1
       9 |      100 |         14.9 |         20.0
       9 |     1000 |         16.1 |         24.4
       9 |    10000 |         21.9 |         25.7
      65 |       70 |         11.7 |         12.5
      65 |      100 |         13.9 |         14.8
      65 |     1000 |         16.7 |         17.8
      65 |    10000 |         22.5 |         27.8
(16 rows)

v2-0001-Deduplicate-entries-in-ResourceOwner.patch:

postgres=# \i contrib/resownerbench/snaptest.sql
 numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
       0 |        1 |         10.8 |         10.6
       0 |        5 |         11.5 |         12.3
       0 |       10 |         12.1 |         13.0
       0 |       60 |         13.9 |         19.4
       0 |       70 |         15.9 |         18.7
       0 |      100 |         16.0 |         18.5
       0 |     1000 |         19.2 |         21.6
       0 |    10000 |         22.4 |         29.0
       9 |       10 |         11.2 |         11.3
       9 |      100 |         14.4 |         19.9
       9 |     1000 |         16.4 |         23.8
       9 |    10000 |         22.4 |         25.7
      65 |       70 |         11.4 |         12.1
      65 |      100 |         14.8 |         17.0
      65 |     1000 |         16.9 |         18.1
      65 |    10000 |         22.5 |         28.5
(16 rows)

v20251016-0001-Deduplicate-entries-in-ResourceOwner.patch:

postgres=# \i contrib/resownerbench/snaptest.sql
 numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
       0 |        1 |         11.3 |         11.1
       0 |        5 |         12.3 |         13.0
       0 |       10 |         13.0 |         14.1
       0 |       60 |         14.7 |         20.5
       0 |       70 |         16.3 |         19.0
       0 |      100 |         16.5 |         18.4
       0 |     1000 |         19.0 |         22.4
       0 |    10000 |         23.2 |         29.6
       9 |       10 |         11.2 |         11.1
       9 |      100 |         14.8 |         20.5
       9 |     1000 |         16.8 |         24.3
       9 |    10000 |         23.3 |         26.5
      65 |       70 |         12.4 |         13.0
      65 |      100 |         15.2 |         16.6
      65 |     1000 |         16.9 |         18.4
      65 |    10000 |         23.4 |         29.3
(16 rows)

These are just a single run on my laptop, the error bars on individual numbers are significant. But it seems to me that V2 is maybe a little faster when the entries fit in the array.

- Heikki
From c19d37c10cbe441fcd272bc67befca11d46fee21 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 21 Oct 2025 09:51:23 +0300
Subject: [PATCH v3 1/1] resownerbench

---
 contrib/Makefile                            |   1 +
 contrib/meson.build                         |   2 +
 contrib/resownerbench/Makefile              |  17 +++
 contrib/resownerbench/meson.build           |  24 +++
 contrib/resownerbench/resownerbench.c       | 154 ++++++++++++++++++++
 contrib/resownerbench/resownerbench.control |   6 +
 contrib/resownerbench/snaptest.sql          |  37 +++++
 7 files changed, 241 insertions(+)
 create mode 100644 contrib/resownerbench/Makefile
 create mode 100644 contrib/resownerbench/meson.build
 create mode 100644 contrib/resownerbench/resownerbench.c
 create mode 100644 contrib/resownerbench/resownerbench.control
 create mode 100644 contrib/resownerbench/snaptest.sql

diff --git a/contrib/Makefile b/contrib/Makefile
index 2f0a88d3f77..f84b26d52cf 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -43,6 +43,7 @@ SUBDIRS = \
 		pg_visibility	\
 		pg_walinspect	\
 		postgres_fdw	\
+		resownerbench	\
 		seg		\
 		spi		\
 		tablefunc	\
diff --git a/contrib/meson.build b/contrib/meson.build
index ed30ee7d639..9c34bb128b7 100644
--- a/contrib/meson.build
+++ b/contrib/meson.build
@@ -71,3 +71,5 @@ subdir('unaccent')
 subdir('uuid-ossp')
 subdir('vacuumlo')
 subdir('xml2')
+
+subdir('resownerbench')
diff --git a/contrib/resownerbench/Makefile b/contrib/resownerbench/Makefile
new file mode 100644
index 00000000000..9b0e1cfee1a
--- /dev/null
+++ b/contrib/resownerbench/Makefile
@@ -0,0 +1,17 @@
+MODULE_big = resownerbench
+OBJS = resownerbench.o
+
+EXTENSION = resownerbench
+DATA = resownerbench--1.0.sql
+PGFILEDESC = "resownerbench - benchmark for ResourceOwners"
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/resownerbench
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/resownerbench/meson.build b/contrib/resownerbench/meson.build
new file mode 100644
index 00000000000..30051c3a375
--- /dev/null
+++ b/contrib/resownerbench/meson.build
@@ -0,0 +1,24 @@
+# Copyright (c) 2022-2025, PostgreSQL Global Development Group
+
+resownerbench_sources = files(
+  'resownerbench.c',
+)
+
+resownerbench = shared_module('resownerbench',
+  resownerbench_sources,
+  c_pch: pch_postgres_h,
+  kwargs: contrib_mod_args,
+)
+contrib_targets += resownerbench
+
+install_data(
+  'resownerbench.control',
+  'resownerbench--1.0.sql',
+  kwargs: contrib_data_args,
+)
+
+tests += {
+  'name': 'resownerbench',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+}
diff --git a/contrib/resownerbench/resownerbench.c b/contrib/resownerbench/resownerbench.c
new file mode 100644
index 00000000000..80f98c2105b
--- /dev/null
+++ b/contrib/resownerbench/resownerbench.c
@@ -0,0 +1,154 @@
+#include "postgres.h"
+
+#include "catalog/pg_type.h"
+#include "catalog/pg_statistic.h"
+#include "executor/spi.h"
+#include "funcapi.h"
+#include "libpq/pqsignal.h"
+#include "utils/catcache.h"
+#include "utils/syscache.h"
+#include "utils/timestamp.h"
+#include "utils/snapmgr.h"
+
+PG_MODULE_MAGIC;
+
+PG_FUNCTION_INFO_V1(snapshotbench_lifo);
+PG_FUNCTION_INFO_V1(snapshotbench_fifo);
+
+/*
+ * ResourceOwner Performance test, using RegisterSnapshot().
+ *
+ * This takes three parameters: numkeep, numsnaps, numiters.
+ *
+ * First, we register 'numkeep' snapshots. They are kept registed
+ * until the end of the test. Then, we repeatedly register and
+ * unregister 'numsnaps - numkeep' additional snapshots, repeating
+ * 'numiters' times. All the register/unregister calls are made in
+ * LIFO order.
+ *
+ * Returns the time spent, in milliseconds.
+ *
+ * The idea is to test the performance of ResourceOwnerRemember()
+ * and ReourceOwnerForget() operations, under different regimes.
+ *
+ * In the old implementation, if 'numsnaps' is small enough, all
+ * the entries fit in the resource owner's small array (it can
+ * hold 64 entries).
+ *
+ * In the new implementation, the array is much smaller, only 8
+ * entries, but it's used together with the hash table so that
+ * we stay in the "array regime" as long as 'numsnaps - numkeep'
+ * is smaller than 8 entries.
+ *
+ * 'numiters' can be adjusted to adjust the overall runtime to be
+ * suitable long.
+ */
+Datum
+snapshotbench_lifo(PG_FUNCTION_ARGS)
+{
+	int			numkeep = PG_GETARG_INT32(0);
+	int			numsnaps = PG_GETARG_INT32(1);
+	int			numiters = PG_GETARG_INT32(2);
+	int			i;
+	instr_time	start,
+				duration;
+	Snapshot	lsnap;
+	Snapshot   *rs;
+	int			numregistered = 0;
+
+	rs = palloc(Max(numsnaps, numkeep) * sizeof(Snapshot));
+
+	lsnap = GetLatestSnapshot();
+
+	sigprocmask(SIG_SETMASK, &BlockSig, NULL);
+	INSTR_TIME_SET_CURRENT(start);
+
+	while (numregistered < numkeep)
+	{
+		rs[numregistered] = RegisterSnapshot(lsnap);
+		numregistered++;
+	}
+
+	for (i = 0 ; i < numiters; i++)
+	{
+		while (numregistered < numsnaps)
+		{
+			rs[numregistered] = RegisterSnapshot(lsnap);
+			numregistered++;
+		}
+
+		while (numregistered > numkeep)
+		{
+			numregistered--;
+			UnregisterSnapshot(rs[numregistered]);
+		}
+	}
+
+	while (numregistered > 0)
+	{
+		numregistered--;
+		UnregisterSnapshot(rs[numregistered]);
+	}
+
+	INSTR_TIME_SET_CURRENT(duration);
+	INSTR_TIME_SUBTRACT(duration, start);
+	sigprocmask(SIG_SETMASK, &UnBlockSig, NULL);
+
+	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(duration));
+};
+
+
+/*
+ * Same, but do the register/unregister operations in
+ * FIFO order.
+ */
+Datum
+snapshotbench_fifo(PG_FUNCTION_ARGS)
+{
+	int			numkeep = PG_GETARG_INT32(0);
+	int			numsnaps = PG_GETARG_INT32(1);
+	int			numiters = PG_GETARG_INT32(2);
+	int			i,
+				j;
+	instr_time	start,
+				duration;
+	Snapshot	lsnap;
+	Snapshot   *rs;
+	int			numregistered = 0;
+
+	rs = palloc(Max(numsnaps, numkeep) * sizeof(Snapshot));
+
+	lsnap = GetLatestSnapshot();
+
+	sigprocmask(SIG_SETMASK, &BlockSig, NULL);
+	INSTR_TIME_SET_CURRENT(start);
+
+	while (numregistered < numkeep)
+	{
+		rs[numregistered] = RegisterSnapshot(lsnap);
+		numregistered++;
+	}
+
+	for (i = 0 ; i < numiters; i++)
+	{
+		while (numregistered < numsnaps)
+		{
+			rs[numregistered] = RegisterSnapshot(lsnap);
+			numregistered++;
+		}
+
+		for (j = numkeep; j < numregistered; j++)
+			UnregisterSnapshot(rs[j]);
+		numregistered = numkeep;
+	}
+
+	for (j = 0; j < numregistered; j++)
+		UnregisterSnapshot(rs[j]);
+	numregistered = numkeep;
+
+	INSTR_TIME_SET_CURRENT(duration);
+	INSTR_TIME_SUBTRACT(duration, start);
+	sigprocmask(SIG_SETMASK, &UnBlockSig, NULL);
+
+	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(duration));
+};
diff --git a/contrib/resownerbench/resownerbench.control b/contrib/resownerbench/resownerbench.control
new file mode 100644
index 00000000000..ada88b8eed8
--- /dev/null
+++ b/contrib/resownerbench/resownerbench.control
@@ -0,0 +1,6 @@
+# resownerbench
+
+comment = 'benchmark for ResourceOwners'
+default_version = '1.0'
+module_pathname = '$libdir/resownerbench'
+relocatable = true
diff --git a/contrib/resownerbench/snaptest.sql b/contrib/resownerbench/snaptest.sql
new file mode 100644
index 00000000000..e3c0a9710ee
--- /dev/null
+++ b/contrib/resownerbench/snaptest.sql
@@ -0,0 +1,37 @@
+--
+-- Performance test RegisterSnapshot/UnregisterSnapshot.
+--
+select numkeep, numsnaps,
+       -- numiters,
+       -- round(lifo_time_ms) as lifo_total_time_ms,
+       -- round(fifo_time_ms) as fifo_total_time_ms,
+       round((lifo_time_ms::numeric / (numkeep + (numsnaps - numkeep) * numiters)) * 1000000, 1) as lifo_time_ns,
+       round((fifo_time_ms::numeric / (numkeep + (numsnaps - numkeep) * numiters)) * 1000000, 1) as fifo_time_ns
+from
+(values (0,      1,  10000000 * 10),
+        (0,      5,   2000000 * 10),
+        (0,     10,   1000000 * 10),
+        (0,     60,    100000 * 10),
+        (0,     70,    100000 * 10),
+        (0,    100,    100000 * 10),
+        (0,   1000,     10000 * 10),
+        (0,  10000,      1000 * 10),
+
+-- These tests keep 9 snapshots registered across the iterations. That
+-- exceeds the size of the little array in the patch, so this exercises
+-- the hash lookups. Without the patch, these still fit in the array
+-- (it's 64 entries without the patch)
+        (9,     10,  10000000 * 10),
+        (9,    100,    100000 * 10),
+        (9,   1000,     10000 * 10),
+        (9,  10000,      1000 * 10),
+
+-- These exceed the 64 entry array even without the patch, so these fall
+-- in the hash table regime with and without the patch.
+        (65,    70,   1000000 * 10),
+        (65,   100,    100000 * 10),
+        (65,  1000,     10000 * 10),
+        (65, 10000,      1000 * 10)
+) AS params (numkeep, numsnaps, numiters),
+lateral snapshotbench_lifo(numkeep, numsnaps, numiters) as lifo_time_ms,
+lateral snapshotbench_fifo(numkeep, numsnaps, numiters) as fifo_time_ms;
-- 
2.47.3

Reply via email to