From d122170f942932f9d747f8437a8089990665457a Mon Sep 17 00:00:00 2001
From: Michael Zhilin <m.zhilin@postgrespro.ru>
Date: Thu, 10 Mar 2022 17:23:11 +0300
Subject: [PATCH 2/2] wip: pg_stat_activity: allocate memory differently

This patch changes the way memory is allocated for temporary (aka local state)
structures.  In the case of large values of max_connections (over 1000), memory
was allocated to accommodate the worst-case number of active backends which may
be wasteful and cause increased latency.

The patch makes pg_stat_activity view a bit more efficient by allocating memory
as needed for active backends.  It's still allocated in chunks.

Co-authored-by: Yura Sokolov <y.sokolov@postgrespro.ru>
---
 src/backend/utils/activity/backend_status.c | 107 ++++++++++++++------
 src/backend/utils/adt/pgstatfuncs.c         |   4 +-
 src/include/utils/backend_status.h          |   2 +-
 3 files changed, 77 insertions(+), 36 deletions(-)

diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 64567ff5cd7..9fbe8a601db 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -25,6 +25,7 @@
 #include "utils/guc.h"			/* for application_name */
 #include "utils/memutils.h"
 
+#define UINT32_ACCESS_ONCE(var)           ((uint32)(*((volatile uint32 *)&(var))))
 
 /* ----------
  * Total number of backends including auxiliary
@@ -716,6 +717,24 @@ pgstat_report_xact_timestamp(TimestampTz tstamp)
 	PGSTAT_END_WRITE_ACTIVITY(beentry);
 }
 
+#define PGSTAT_LOCALDATA_CHUNK	128
+#ifdef HAVE_TYPEOF
+#define PGSTAT_LOCALDATA_ALLOC(x,cnt) \
+	if (x == NULL || x > x##_thr) {	\
+		x = (typeof(x)) \
+			MemoryContextAlloc(backendStatusSnapContext, \
+							   cnt * sizeof(x[0]) * PGSTAT_LOCALDATA_CHUNK); \
+		x##_thr = x + cnt * (PGSTAT_LOCALDATA_CHUNK - 1); \
+	}
+#else
+#define PGSTAT_LOCALDATA_ALLOC(x,cnt) \
+	if (x == NULL || x > x##_thr) {	\
+		x = MemoryContextAlloc(backendStatusSnapContext, \
+							   cnt * sizeof(x[0]) * PGSTAT_LOCALDATA_CHUNK); \
+		x##_thr = x + cnt * (PGSTAT_LOCALDATA_CHUNK - 1); \
+	}
+#endif
+
 /* ----------
  * pgstat_read_current_status() -
  *
@@ -729,16 +748,24 @@ pgstat_read_current_status(void)
 	volatile PgBackendStatus *beentry;
 	LocalPgBackendStatus *localtable;
 	LocalPgBackendStatus *localentry;
+	PgBackendStatus *localbeentry,
+			   *localbeentry_thr;
 	char	   *localappname,
+			   *localappname_thr,
 			   *localclienthostname,
-			   *localactivity;
+			   *localclienthostname_thr,
+			   *localactivity,
+			   *localactivity_thr;
 #ifdef USE_SSL
-	PgBackendSSLStatus *localsslstatus;
+	PgBackendSSLStatus *localsslstatus,
+			   *localsslstatus_thr;
 #endif
 #ifdef ENABLE_GSS
-	PgBackendGSSStatus *localgssstatus;
+	PgBackendGSSStatus *localgssstatus,
+			   *localgssstatus_thr;
 #endif
 	int			i;
+	int			pid;
 
 	if (localBackendStatusTable)
 		return;					/* already done */
@@ -756,32 +783,39 @@ pgstat_read_current_status(void)
 	localtable = (LocalPgBackendStatus *)
 		MemoryContextAlloc(backendStatusSnapContext,
 						   sizeof(LocalPgBackendStatus) * NumBackendStatSlots);
-	localappname = (char *)
-		MemoryContextAlloc(backendStatusSnapContext,
-						   NAMEDATALEN * NumBackendStatSlots);
-	localclienthostname = (char *)
-		MemoryContextAlloc(backendStatusSnapContext,
-						   NAMEDATALEN * NumBackendStatSlots);
-	localactivity = (char *)
-		MemoryContextAllocHuge(backendStatusSnapContext,
-							   pgstat_track_activity_query_size * NumBackendStatSlots);
+
 #ifdef USE_SSL
-	localsslstatus = (PgBackendSSLStatus *)
-		MemoryContextAlloc(backendStatusSnapContext,
-						   sizeof(PgBackendSSLStatus) * NumBackendStatSlots);
+	localsslstatus = NULL;
 #endif
 #ifdef ENABLE_GSS
-	localgssstatus = (PgBackendGSSStatus *)
-		MemoryContextAlloc(backendStatusSnapContext,
-						   sizeof(PgBackendGSSStatus) * NumBackendStatSlots);
+	localgssstatus = NULL;
 #endif
 
 	localNumBackends = 0;
+	localbeentry = NULL;
+	localappname = NULL;
+	localclienthostname = NULL;
+	localactivity = NULL;
 
 	beentry = BackendStatusArray;
 	localentry = localtable;
 	for (i = 1; i <= NumBackendStatSlots; i++)
 	{
+		/*
+		 * Check available space in buffers for local state data
+		 */
+		PGSTAT_LOCALDATA_ALLOC(localbeentry, 1);
+		PGSTAT_LOCALDATA_ALLOC(localappname, NAMEDATALEN);
+		PGSTAT_LOCALDATA_ALLOC(localclienthostname, NAMEDATALEN);
+		PGSTAT_LOCALDATA_ALLOC(localactivity, pgstat_track_activity_query_size);
+#ifdef USE_SSL
+		PGSTAT_LOCALDATA_ALLOC(localsslstatus, 1);
+#endif
+#ifdef ENABLE_GSS
+		PGSTAT_LOCALDATA_ALLOC(localgssstatus, 1);
+#endif
+
+		localentry->backendStatus = localbeentry;
 		/*
 		 * Follow the protocol of retrying if st_changecount changes while we
 		 * copy the entry, or if it's odd.  (The check for odd is needed to
@@ -796,11 +830,11 @@ pgstat_read_current_status(void)
 
 			pgstat_begin_read_activity(beentry, before_changecount);
 
-			localentry->backendStatus.st_procpid = beentry->st_procpid;
+			localentry->backendStatus->st_procpid = beentry->st_procpid;
 			/* Skip all the data-copying work if entry is not in use */
-			if (localentry->backendStatus.st_procpid > 0)
+			if (localentry->backendStatus->st_procpid > 0)
 			{
-				memcpy(&localentry->backendStatus, unvolatize(PgBackendStatus *, beentry), sizeof(PgBackendStatus));
+				memcpy(localentry->backendStatus, unvolatize(PgBackendStatus *, beentry), sizeof(PgBackendStatus));
 
 				/*
 				 * For each PgBackendStatus field that is a pointer, copy the
@@ -811,23 +845,23 @@ pgstat_read_current_status(void)
 				 * because there's always a \0 at the end of the buffer.
 				 */
 				strcpy(localappname, (char *) beentry->st_appname);
-				localentry->backendStatus.st_appname = localappname;
+				localentry->backendStatus->st_appname = localappname;
 				strcpy(localclienthostname, (char *) beentry->st_clienthostname);
-				localentry->backendStatus.st_clienthostname = localclienthostname;
+				localentry->backendStatus->st_clienthostname = localclienthostname;
 				strcpy(localactivity, (char *) beentry->st_activity_raw);
-				localentry->backendStatus.st_activity_raw = localactivity;
+				localentry->backendStatus->st_activity_raw = localactivity;
 #ifdef USE_SSL
 				if (beentry->st_ssl)
 				{
 					memcpy(localsslstatus, beentry->st_sslstatus, sizeof(PgBackendSSLStatus));
-					localentry->backendStatus.st_sslstatus = localsslstatus;
+					localentry->backendStatus->st_sslstatus = localsslstatus;
 				}
 #endif
 #ifdef ENABLE_GSS
 				if (beentry->st_gss)
 				{
 					memcpy(localgssstatus, beentry->st_gssstatus, sizeof(PgBackendGSSStatus));
-					localentry->backendStatus.st_gssstatus = localgssstatus;
+					localentry->backendStatus->st_gssstatus = localgssstatus;
 				}
 #endif
 			}
@@ -842,8 +876,9 @@ pgstat_read_current_status(void)
 			CHECK_FOR_INTERRUPTS();
 		}
 
+		pid = localentry->backendStatus->st_procpid;
 		/* Only valid entries get included into the local array */
-		if (localentry->backendStatus.st_procpid > 0)
+		if (pid > 0)
 		{
 			PGPROC	*proc = BackendIdGetProc(i);
 			if (proc != NULL)
@@ -855,12 +890,17 @@ pgstat_read_current_status(void)
 				 * leaves the field as NULL for the leader of a parallel
 				 * group.
 				 */
-				if (leader && leader->pid != localentry->backendStatus.st_procpid)
+				if (leader && leader->pid != localentry->backendStatus->st_procpid)
 					localentry->leader_pid = leader->pid;
 				else
 					localentry->leader_pid = 0;
 
-				localentry->wait_event_info = proc->wait_event_info;
+				localentry->wait_event_info = UINT32_ACCESS_ONCE(proc->wait_event_info);
+			}
+			else
+			{
+				localentry->leader_pid = 0;
+				localentry->wait_event_info = 0;
 			}
 
 			BackendIdGetTransactionIds(i,
@@ -868,9 +908,10 @@ pgstat_read_current_status(void)
 									   &localentry->backend_xmin);
 
 			localentry++;
-			localappname += NAMEDATALEN;
-			localclienthostname += NAMEDATALEN;
-			localactivity += pgstat_track_activity_query_size;
+			localbeentry++;
+			localappname += strlen(localappname) + 1;
+			localclienthostname += strlen(localclienthostname) + 1;
+			localactivity += strlen(localactivity) + 1;
 #ifdef USE_SSL
 			localsslstatus++;
 #endif
@@ -1083,7 +1124,7 @@ pgstat_fetch_stat_beentry(int beid)
 	if (beid < 1 || beid > localNumBackends)
 		return NULL;
 
-	return &localBackendStatusTable[beid - 1].backendStatus;
+	return localBackendStatusTable[beid - 1].backendStatus;
 }
 
 
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 82f169a2d8b..d5f2448912b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -500,7 +500,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 		if (!local_beentry)
 			continue;
 
-		beentry = &local_beentry->backendStatus;
+		beentry = local_beentry->backendStatus;
 
 		/*
 		 * Report values for only those backends which are running the given
@@ -581,7 +581,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			continue;
 		}
 
-		beentry = &local_beentry->backendStatus;
+		beentry = local_beentry->backendStatus;
 
 		/* If looking for specific PID, ignore all the others */
 		if (pid != -1 && beentry->st_procpid != pid)
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 8f890911486..0b4b426e3aa 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -245,7 +245,7 @@ typedef struct LocalPgBackendStatus
 	/*
 	 * Local version of the backend status entry.
 	 */
-	PgBackendStatus backendStatus;
+	PgBackendStatus *backendStatus;
 
 	/*
 	 * The xid of the current transaction if available, InvalidTransactionId
-- 
2.17.1

