From ffe65b418be46a8b4d9f5e51891967abbff96f86 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Mon, 1 Sep 2025 10:54:35 +0000 Subject: [PATCH v3] Get rid of pgstat_count_backend_io_op*() functions This commit removes the functions that are incrementing the backend IO stats. Instead, it now copies the IO pending stats to the backend IO pending stats when the pending IO stats are flushed. This behaves the same way as for some relation and database stats. It's done that way to avoid incrementing the "same" counters twice as it produces increased overhead in profiles. Note that per-backend statistics have to be last in pgstat_report_stat() as some of their pending stats are populated during other stats kinds flush. Reported-by: Andres Freund --- src/backend/utils/activity/pgstat.c | 14 ++++++- src/backend/utils/activity/pgstat_backend.c | 42 +-------------------- src/backend/utils/activity/pgstat_io.c | 18 +++++---- src/include/pgstat.h | 14 +++---- 4 files changed, 31 insertions(+), 57 deletions(-) 81.2% src/backend/utils/activity/ 18.7% src/include/ diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 73c2ced3f4e..f57a66d89b0 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -768,10 +768,15 @@ pgstat_report_stat(bool force) /* flush of other stats kinds */ if (pgstat_report_fixed) { + const PgStat_KindInfo *kind_info; + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) { - const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + kind_info = pgstat_get_kind_info(kind); + + if (kind == PGSTAT_KIND_BACKEND) + continue; if (!kind_info) continue; if (!kind_info->flush_static_cb) @@ -779,6 +784,13 @@ pgstat_report_stat(bool force) partial_flush |= kind_info->flush_static_cb(nowait); } + + /* + * Do per-backend last as some of their pending stats are populated + * during the flush of other stats kinds. + */ + kind_info = pgstat_get_kind_info(PGSTAT_KIND_BACKEND); + partial_flush |= kind_info->flush_static_cb(nowait); } last_flush = now; diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index 07a1116671b..ea80053304d 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -36,8 +36,8 @@ * reported within critical sections so we use static memory in order to avoid * memory allocation. */ -static PgStat_BackendPending PendingBackendStats; -static bool backend_has_iostats = false; +PgStat_BackendPending PendingBackendStats; +bool backend_has_iostats = false; /* * WAL usage counters saved from pgWalUsage at the previous call to @@ -47,44 +47,6 @@ static bool backend_has_iostats = false; */ static WalUsage prevBackendWalUsage; -/* - * Utility routines to report I/O stats for backends, kept here to avoid - * exposing PendingBackendStats to the outside world. - */ -void -pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context, - IOOp io_op, instr_time io_time) -{ - Assert(track_io_timing || track_wal_io_timing); - - if (!pgstat_tracks_backend_bktype(MyBackendType)) - return; - - Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op)); - - INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op], - io_time); - - backend_has_iostats = true; - pgstat_report_fixed = true; -} - -void -pgstat_count_backend_io_op(IOObject io_object, IOContext io_context, - IOOp io_op, uint32 cnt, uint64 bytes) -{ - if (!pgstat_tracks_backend_bktype(MyBackendType)) - return; - - Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op)); - - PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt; - PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes; - - backend_has_iostats = true; - pgstat_report_fixed = true; -} - /* * Returns statistics of a backend by proc number. */ diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 13ae57ed649..0520f4a1d33 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -76,9 +76,6 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, PendingIOStats.counts[io_object][io_context][io_op] += cnt; PendingIOStats.bytes[io_object][io_context][io_op] += bytes; - /* Add the per-backend counts */ - pgstat_count_backend_io_op(io_object, io_context, io_op, cnt, bytes); - have_iostats = true; pgstat_report_fixed = true; } @@ -151,10 +148,6 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op, INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op], io_time); - - /* Add the per-backend count */ - pgstat_count_backend_io_op_time(io_object, io_context, io_op, - io_time); } pgstat_count_io_op(io_object, io_context, io_op, cnt, bytes); @@ -184,6 +177,9 @@ pgstat_flush_io(bool nowait) * * If nowait is true, this function returns true if the lock could not be * acquired. Otherwise, return false. + * + * The stats are copied to the corresponding pending backend stats when + * successfully flushing. */ bool pgstat_io_flush_cb(bool nowait) @@ -227,6 +223,14 @@ pgstat_io_flush_cb(bool nowait) Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)); + /* Do the same for the backend stats */ + if (pgstat_tracks_backend_bktype(MyBackendType)) + { + PendingBackendStats.pending_io = PendingIOStats; + backend_has_iostats = true; + pgstat_report_fixed = true; + } + LWLockRelease(bktype_lock); memset(&PendingIOStats, 0, sizeof(PendingIOStats)); diff --git a/src/include/pgstat.h b/src/include/pgstat.h index f402b17295c..fe0fdaf4883 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -548,15 +548,6 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void); * Functions in pgstat_backend.c */ -/* used by pgstat_io.c for I/O stats tracked in backends */ -extern void pgstat_count_backend_io_op_time(IOObject io_object, - IOContext io_context, - IOOp io_op, - instr_time io_time); -extern void pgstat_count_backend_io_op(IOObject io_object, - IOContext io_context, - IOOp io_op, uint32 cnt, - uint64 bytes); extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber); extern PgStat_Backend *pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype); @@ -800,6 +791,11 @@ extern PGDLLIMPORT bool pgstat_track_counts; extern PGDLLIMPORT int pgstat_track_functions; extern PGDLLIMPORT int pgstat_fetch_consistency; +/* + * Variables in pgstat_backend.c + */ +extern PGDLLIMPORT PgStat_BackendPending PendingBackendStats; +extern PGDLLIMPORT bool backend_has_iostats; /* * Variables in pgstat_bgwriter.c -- 2.34.1