[Tristan Partin <tristan@neon.tech>]

Instead of needing to be explicit, we can just iterate the
pgstat_kind_infos array to find the memory locations to read into.

This was originally thought of by Andres in
5891c7a8ed8f2d3d577e7eea34dacff12d7b6bbd.

Not a fix, per se, but it does remove a comment. Perhaps the discussion 
will just lead to someone deleting the comment, and keeping the code 
as is. Either way, a win :).

-- 
Tristan Partin
Neon (https://neon.tech)

Attachments:

v1-0001-Use-pgstat_kind_infos-to-read-fixed-shared-stats-.patch (text/x-patch, 5.1K) download
From 5cc5a67edbd3dee145ea582c024b6ee207ae4085 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 6 May 2024 13:52:58 -0500
Subject: [PATCH v1] Use pgstat_kind_infos to read fixed shared stats structs

Instead of needing to be explicit, we can just iterate the
pgstat_kind_infos array to find the memory locations to read into.

This was originally thought of by Andres in
5891c7a8ed8f2d3d577e7eea34dacff12d7b6bbd.
---
 src/backend/utils/activity/pgstat.c | 75 ++++++++++++++---------------
 src/include/utils/pgstat_internal.h |  6 +++
 2 files changed, 42 insertions(+), 39 deletions(-)

diff --git ./src/backend/utils/activity/pgstat.c incoming/src/backend/utils/activity/pgstat.c
index dcc2ad8d954..81e3d702ccd 100644
--- ./src/backend/utils/activity/pgstat.c
+++ incoming/src/backend/utils/activity/pgstat.c
@@ -342,6 +342,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, archiver),
+		.shared_data_off = offsetof(PgStatShared_Archiver, stats),
+		.shared_data_len = sizeof(((PgStatShared_Archiver *) 0)->stats),
+
 		.reset_all_cb = pgstat_archiver_reset_all_cb,
 		.snapshot_cb = pgstat_archiver_snapshot_cb,
 	},
@@ -351,6 +355,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, bgwriter),
+		.shared_data_off = offsetof(PgStatShared_BgWriter, stats),
+		.shared_data_len = sizeof(((PgStatShared_BgWriter *) 0)->stats),
+
 		.reset_all_cb = pgstat_bgwriter_reset_all_cb,
 		.snapshot_cb = pgstat_bgwriter_snapshot_cb,
 	},
@@ -360,6 +368,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, checkpointer),
+		.shared_data_off = offsetof(PgStatShared_Checkpointer, stats),
+		.shared_data_len = sizeof(((PgStatShared_Checkpointer *) 0)->stats),
+
 		.reset_all_cb = pgstat_checkpointer_reset_all_cb,
 		.snapshot_cb = pgstat_checkpointer_snapshot_cb,
 	},
@@ -369,6 +381,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, io),
+		.shared_data_off = offsetof(PgStatShared_IO, stats),
+		.shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats),
+
 		.reset_all_cb = pgstat_io_reset_all_cb,
 		.snapshot_cb = pgstat_io_snapshot_cb,
 	},
@@ -378,6 +394,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, slru),
+		.shared_data_off = offsetof(PgStatShared_SLRU, stats),
+		.shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats),
+
 		.reset_all_cb = pgstat_slru_reset_all_cb,
 		.snapshot_cb = pgstat_slru_snapshot_cb,
 	},
@@ -387,6 +407,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 
 		.fixed_amount = true,
 
+		.shared_ctl_off = offsetof(PgStat_ShmemControl, wal),
+		.shared_data_off = offsetof(PgStatShared_Wal, stats),
+		.shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats),
+
 		.reset_all_cb = pgstat_wal_reset_all_cb,
 		.snapshot_cb = pgstat_wal_snapshot_cb,
 	},
@@ -1520,47 +1544,20 @@ pgstat_read_statsfile(void)
 		format_id != PGSTAT_FILE_FORMAT_ID)
 		goto error;
 
-	/*
-	 * XXX: The following could now be generalized to just iterate over
-	 * pgstat_kind_infos instead of knowing about the different kinds of
-	 * stats.
-	 */
+	/* Read various stats structs */
+	for (PgStat_Kind kind = PGSTAT_KIND_FIRST_VALID; kind < PGSTAT_NUM_KINDS; ++kind)
+	{
+		char	   *ptr;
+		const PgStat_KindInfo *info;
 
-	/*
-	 * Read archiver stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->archiver.stats))
-		goto error;
+		info = pgstat_kind_infos + kind;
+		if (!info->fixed_amount)
+			continue;
 
-	/*
-	 * Read bgwriter stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->bgwriter.stats))
-		goto error;
-
-	/*
-	 * Read checkpointer stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->checkpointer.stats))
-		goto error;
-
-	/*
-	 * Read IO stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->io.stats))
-		goto error;
-
-	/*
-	 * Read SLRU stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->slru.stats))
-		goto error;
-
-	/*
-	 * Read WAL stats struct
-	 */
-	if (!read_chunk_s(fpin, &shmem->wal.stats))
-		goto error;
+		ptr = ((char *) shmem) + info->shared_ctl_off + info->shared_data_off;
+		if (!read_chunk(fpin, ptr, info->shared_data_len))
+			goto error;
+	}
 
 	/*
 	 * We found an existing statistics file. Read it and put all the hash
diff --git ./src/include/utils/pgstat_internal.h incoming/src/include/utils/pgstat_internal.h
index dbbca316025..240c4748040 100644
--- ./src/include/utils/pgstat_internal.h
+++ incoming/src/include/utils/pgstat_internal.h
@@ -199,6 +199,12 @@ typedef struct PgStat_KindInfo
 	 */
 	bool		named_on_disk:1;
 
+	/*
+	 * The offset of the struct in the containing shared memory control
+	 * structure.
+	 */
+	uint32		shared_ctl_off;
+
 	/*
 	 * The size of an entry in the shared stats hash table (pointed to by
 	 * PgStatShared_HashEntry->body).
-- 
Tristan Partin
Neon (https://neon.tech)