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)
From 5cc5a67edbd3dee145ea582c024b6ee207ae4085 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@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)
