From edd45c3aa0731a9645dbcfe0475e40c642e92269 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Wed, 6 Nov 2024 16:19:44 +0000 Subject: [PATCH v5 2/4] Merge both IO stats flush callbacks There is no need to keep both callbacks. Merging both allows to save O(N^3) while looping on IOOBJECT_NUM_TYPES, IOCONTEXT_NUM_TYPES and IOCONTEXT_NUM_TYPES. --- src/backend/utils/activity/pgstat.c | 2 - src/backend/utils/activity/pgstat_io.c | 97 ++++++-------------------- 2 files changed, 23 insertions(+), 76 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index aacf61c9a4..d051db5c10 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -435,8 +435,6 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_off = offsetof(PgStatShared_IO, stats), .shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats), - .flush_fixed_cb = pgstat_io_flush_cb, - .have_fixed_pending_cb = pgstat_io_have_pending_cb, .init_shmem_cb = pgstat_io_init_shmem_cb, .reset_all_cb = pgstat_io_reset_all_cb, .snapshot_cb = pgstat_io_snapshot_cb, diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index a2bb11adc9..1384f8103e 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -21,10 +21,6 @@ #include "utils/pgstat_internal.h" -static PgStat_PendingIO PendingIOStats; -static bool have_iostats = false; - - /* * Check that stats have not been counted for any combination of IOObject, * IOContext, and IOOp which are not tracked for the passed-in BackendType. If @@ -83,11 +79,7 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3 Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op)); entry_ref = pgstat_prep_per_backend_pending(MyProcNumber); - - PendingIOStats.counts[io_object][io_context][io_op] += cnt; entry_ref->counts[io_object][io_context][io_op] += cnt; - - have_iostats = true; } /* @@ -148,8 +140,6 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op, INSTR_TIME_ADD(pgBufferUsage.local_blk_read_time, io_time); } - INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op], - io_time); INSTR_TIME_ADD(entry_ref->pending_times[io_object][io_context][io_op], io_time); } @@ -173,16 +163,7 @@ pgstat_fetch_my_stat_io(void) } /* - * Check if there any IO stats waiting for flush. - */ -bool -pgstat_io_have_pending_cb(void) -{ - return have_iostats; -} - -/* - * Simpler wrapper of pgstat_io_flush_cb() and pgstat_per_backend_flush_cb(). + * Simpler wrapper of pgstat_per_backend_flush_cb(). */ void pgstat_flush_io(bool nowait) @@ -192,81 +173,39 @@ pgstat_flush_io(bool nowait) entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_PER_BACKEND, InvalidOid, MyProcNumber, false, NULL); - (void) pgstat_io_flush_cb(nowait); (void) pgstat_per_backend_flush_cb(entry_ref, nowait); } /* - * Flush out locally pending IO statistics + * Flush out locally pending backend statistics * * If no stats have been recorded, this function returns false. - * - * If nowait is true, this function returns true if the lock could not be - * acquired. Otherwise, return false. */ bool -pgstat_io_flush_cb(bool nowait) +pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) { - LWLock *bktype_lock; + PgStatShared_Backend *shbackendioent; + PgStat_PendingIO *pendingent; PgStat_BktypeIO *bktype_shstats; + LWLock *bktype_lock; + PgStat_BktypeIO *bktype_global_shstats; - if (!have_iostats) + if (!pgstat_lock_entry(entry_ref, nowait)) return false; + /* global IO stats */ bktype_lock = &pgStatLocal.shmem->io.locks[MyBackendType]; - bktype_shstats = - &pgStatLocal.shmem->io.stats.stats[MyBackendType]; + bktype_global_shstats = &pgStatLocal.shmem->io.stats.stats[MyBackendType]; if (!nowait) LWLockAcquire(bktype_lock, LW_EXCLUSIVE); else if (!LWLockConditionalAcquire(bktype_lock, LW_EXCLUSIVE)) - return true; - - for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++) { - for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++) - { - for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++) - { - instr_time time; - - bktype_shstats->counts[io_object][io_context][io_op] += - PendingIOStats.counts[io_object][io_context][io_op]; - - time = PendingIOStats.pending_times[io_object][io_context][io_op]; - - bktype_shstats->times[io_object][io_context][io_op] += - INSTR_TIME_GET_MICROSEC(time); - } - } - } - - Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)); - - LWLockRelease(bktype_lock); - - memset(&PendingIOStats, 0, sizeof(PendingIOStats)); - - have_iostats = false; - - return false; -} - -/* - * Flush out locally pending backend statistics - * - * If no stats have been recorded, this function returns false. - */ -bool -pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) -{ - PgStatShared_Backend *shbackendioent; - PgStat_PendingIO *pendingent; - PgStat_BktypeIO *bktype_shstats; - - if (!pgstat_lock_entry(entry_ref, nowait)) + pgstat_unlock_entry(entry_ref); return false; + } + /* per backend IO stats */ shbackendioent = (PgStatShared_Backend *) entry_ref->shared_stats; bktype_shstats = &shbackendioent->stats.stats; pendingent = (PgStat_PendingIO *) entry_ref->pending; @@ -279,6 +218,7 @@ pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) { instr_time time; + /* per backend IO stats */ bktype_shstats->counts[io_object][io_context][io_op] += pendingent->counts[io_object][io_context][io_op]; @@ -286,10 +226,19 @@ pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) bktype_shstats->times[io_object][io_context][io_op] += INSTR_TIME_GET_MICROSEC(time); + + /* global IO stats */ + bktype_global_shstats->counts[io_object][io_context][io_op] += + pendingent->counts[io_object][io_context][io_op]; + + bktype_global_shstats->times[io_object][io_context][io_op] += + INSTR_TIME_GET_MICROSEC(time); } } } + LWLockRelease(bktype_lock); + pgstat_unlock_entry(entry_ref); return true; -- 2.34.1