Hi all, While reading again the code of injection_stats_fixed.c that holds the template for fixed-numbered pgstats, I got an idea to make the code a bit more elegant. Rather than using a single routine to increment the counters, we could use a series of routines with its internals hidden in a macro like in pgstatfuncs.c.
The point of a template is to be as elegant as possible, and this is arguably a better style, still this comes down to personal taste perhaps. Please feel free to comment about the attached. Thanks, -- Michael
From caa28aecf2726ffd0b2161e56ac832a3c1590469 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 19 Aug 2024 09:23:28 +0900
Subject: [PATCH] injection_points: Make template for fixed-numbered stats more
elegant
There is no functional change here, but the point of a template is to be
be as clear as possible as it should be a point of reference, and this
is arguably more elegant long-term.
---
.../injection_points/injection_points.c | 10 ++--
.../injection_points/injection_stats.h | 10 ++--
.../injection_points/injection_stats_fixed.c | 55 ++++++++++---------
3 files changed, 40 insertions(+), 35 deletions(-)
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 4e775c7ec6..d8cfde2bf5 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -297,7 +297,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
condition.pid = MyProcPid;
}
- pgstat_report_inj_fixed(1, 0, 0, 0, 0);
+ pgstat_report_inj_fixed_numattach(1);
InjectionPointAttach(name, "injection_points", function, &condition,
sizeof(InjectionPointCondition));
@@ -329,7 +329,7 @@ injection_points_load(PG_FUNCTION_ARGS)
if (inj_state == NULL)
injection_init_shmem();
- pgstat_report_inj_fixed(0, 0, 0, 0, 1);
+ pgstat_report_inj_fixed_numloaded(1);
INJECTION_POINT_LOAD(name);
PG_RETURN_VOID();
@@ -344,7 +344,7 @@ injection_points_run(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
- pgstat_report_inj_fixed(0, 0, 1, 0, 0);
+ pgstat_report_inj_fixed_numrun(1);
INJECTION_POINT(name);
PG_RETURN_VOID();
@@ -359,7 +359,7 @@ injection_points_cached(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
- pgstat_report_inj_fixed(0, 0, 0, 1, 0);
+ pgstat_report_inj_fixed_numcached(1);
INJECTION_POINT_CACHED(name);
PG_RETURN_VOID();
@@ -436,7 +436,7 @@ injection_points_detach(PG_FUNCTION_ARGS)
{
char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
- pgstat_report_inj_fixed(0, 1, 0, 0, 0);
+ pgstat_report_inj_fixed_numdetach(1);
if (!InjectionPointDetach(name))
elog(ERROR, "could not detach injection point \"%s\"", name);
diff --git a/src/test/modules/injection_points/injection_stats.h b/src/test/modules/injection_points/injection_stats.h
index 126c110169..ff9fcde0e3 100644
--- a/src/test/modules/injection_points/injection_stats.h
+++ b/src/test/modules/injection_points/injection_stats.h
@@ -23,10 +23,10 @@ extern void pgstat_report_inj(const char *name);
/* injection_stats_fixed.c */
extern void pgstat_register_inj_fixed(void);
-extern void pgstat_report_inj_fixed(uint32 numattach,
- uint32 numdetach,
- uint32 numrun,
- uint32 numcached,
- uint32 numloaded);
+extern void pgstat_report_inj_fixed_numattach(uint32 numattach);
+extern void pgstat_report_inj_fixed_numdetach(uint32 numdetach);
+extern void pgstat_report_inj_fixed_numrun(uint32 numrun);
+extern void pgstat_report_inj_fixed_numcached(uint32 numcached);
+extern void pgstat_report_inj_fixed_numloaded(uint32 numloaded);
#endif
diff --git a/src/test/modules/injection_points/injection_stats_fixed.c b/src/test/modules/injection_points/injection_stats_fixed.c
index 82b07e5332..269dcc428b 100644
--- a/src/test/modules/injection_points/injection_stats_fixed.c
+++ b/src/test/modules/injection_points/injection_stats_fixed.c
@@ -133,33 +133,38 @@ pgstat_register_inj_fixed(void)
inj_fixed_loaded = true;
}
-/*
- * Report fixed number of statistics for an injection point.
- */
-void
-pgstat_report_inj_fixed(uint32 numattach,
- uint32 numdetach,
- uint32 numrun,
- uint32 numcached,
- uint32 numloaded)
-{
- PgStatShared_InjectionPointFixed *stats_shmem;
-
- /* leave if disabled */
- if (!inj_fixed_loaded)
- return;
-
- stats_shmem = pgstat_get_custom_shmem_data(PGSTAT_KIND_INJECTION_FIXED);
-
- pgstat_begin_changecount_write(&stats_shmem->changecount);
- stats_shmem->stats.numattach += numattach;
- stats_shmem->stats.numdetach += numdetach;
- stats_shmem->stats.numrun += numrun;
- stats_shmem->stats.numcached += numcached;
- stats_shmem->stats.numloaded += numloaded;
- pgstat_end_changecount_write(&stats_shmem->changecount);
+#define PGSTAT_REPORT_INJ_FIXED_COUNTER(numstat) \
+void \
+CppConcat(pgstat_report_inj_fixed_,numstat)(uint32 numstat) \
+{ \
+ PgStatShared_InjectionPointFixed *stats_shmem; \
+ \
+ /* leave if disabled */ \
+ if (!inj_fixed_loaded) \
+ return; \
+ \
+ stats_shmem = pgstat_get_custom_shmem_data(PGSTAT_KIND_INJECTION_FIXED); \
+ \
+ pgstat_begin_changecount_write(&stats_shmem->changecount); \
+ stats_shmem->stats.numstat += numstat; \
+ pgstat_end_changecount_write(&stats_shmem->changecount); \
}
+/* pgstat_report_inj_fixed_numattach */
+PGSTAT_REPORT_INJ_FIXED_COUNTER(numattach)
+
+/* pgstat_report_inj_fixed_numdetach */
+PGSTAT_REPORT_INJ_FIXED_COUNTER(numdetach)
+
+/* pgstat_report_inj_fixed_numrun */
+PGSTAT_REPORT_INJ_FIXED_COUNTER(numrun)
+
+/* pgstat_report_inj_fixed_numcached */
+PGSTAT_REPORT_INJ_FIXED_COUNTER(numcached)
+
+/* pgstat_report_inj_fixed_numloaded */
+PGSTAT_REPORT_INJ_FIXED_COUNTER(numloaded)
+
/*
* SQL function returning fixed-numbered statistics for injection points.
*/
--
2.45.2
signature.asc
Description: PGP signature
