From dfe00b07d345938ce393f0dc90e534aa845b8c65 Mon Sep 17 00:00:00 2001 From: Dmitry Fomin Date: Fri, 18 Sep 2026 12:24:38 +0200 Subject: [PATCH v14 1/5] Add begin/end hooks for timed wait events The wait event machinery reports which wait a backend is in, but not how long it took. Measuring that in core would mean paying for a timestamp at every wait site; leaving it to an extension means giving the extension a place to stand. Add wait_event_begin_hook and wait_event_end_hook, called from a new pair of reporting functions, pgstat_report_wait_start_timed() and pgstat_report_wait_end_timed(), which an instrumented call site uses in place of the ordinary pair. The ordinary pgstat_report_wait_start() and pgstat_report_wait_end() are left exactly as they were, so an uninstrumented site, and any build with no extension loaded, behaves as it does today. The hooks run in the middle of the wait path, where a backend may hold an LWLock, sit inside a critical section, or belong to a process with no transaction at all. A hook implementation may therefore use only state it has preallocated for itself: it must not palloc(), acquire a lock, wait, or raise an error. wait_event_hook_depth guards against re-entry, so a hook that breaks that rule and waits is not called recursively. A consumer that chains onto a hook another extension has already installed calls the previous begin hook before its own begin work, and its own end work before the previous end hook, so that nested consumers unwind in the order they were installed. With no hook installed, the cost added at an instrumented site is a predictable test of a NULL function pointer. The enabled path lives in two cold out-of-line functions in wait_event.c; the inline pair is one unlikely()-hinted pointer test per side, the hint chosen because GCC's static predictor treats a pointer compared with NULL as non-NULL; recorded data, hook contract and the ordinary pair are unchanged. The hook pointers are kept in one struct with the my_wait_event_info pointer, so the inline test uses the base address the wait event store already needs; this keeps GCC from holding a separate hook address over loops in callers such as FileReadV(). The previous names remain as macros. Discussion: https://postgr.es/m/CAPHG-0mAOn05ae6Kqx1wHXxzOk4E5W7ajjd=QBhgkR7a0uyQmw@mail.gmail.com --- src/backend/storage/lmgr/s_lock.c | 2 +- src/backend/utils/activity/wait_event.c | 58 +++++++++++++++++- src/include/utils/wait_event.h | 81 ++++++++++++++++++++++++- src/tools/pgindent/typedefs.list | 1 + 4 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c index 795aeed1028..4e9c8fad8ac 100644 --- a/src/backend/storage/lmgr/s_lock.c +++ b/src/backend/storage/lmgr/s_lock.c @@ -66,7 +66,7 @@ * s_lock_test. */ static uint32 local_my_wait_event_info; -uint32 *my_wait_event_info = &local_my_wait_event_info; +WaitEventReportState wait_event_report_state = {&local_my_wait_event_info, NULL, NULL}; #endif static int spins_per_delay = DEFAULT_SPINS_PER_DELAY; diff --git a/src/backend/utils/activity/wait_event.c b/src/backend/utils/activity/wait_event.c index e36a740a888..2206ac11040 100644 --- a/src/backend/utils/activity/wait_event.c +++ b/src/backend/utils/activity/wait_event.c @@ -38,7 +38,63 @@ static const char *pgstat_get_wait_io(WaitEventIO w); static uint32 local_my_wait_event_info; -uint32 *my_wait_event_info = &local_my_wait_event_info; +WaitEventReportState wait_event_report_state = {&local_my_wait_event_info, NULL, NULL}; + +int wait_event_hook_depth = 0; + +/* + * Slow paths for pgstat_report_wait_start_timed() and + * pgstat_report_wait_end_timed() in wait_event.h, reached only once a hook + * has actually been installed in this process. Keeping them out of line + * and marked cold lets the inline callers stay down to a single hinted + * pointer test on the common no-consumer path. + * + * Each function re-reads its hook pointer rather than being handed the + * value the inline caller already loaded. The hook cannot actually change + * between the inline test and this call -- both run in the same process, + * and there is no window for another backend to reach in -- but re-reading + * it here keeps that assumption local to this file instead of leaking into + * the header. + * + * Both functions save and restore errno around the hook call: callers look + * at errno after pgstat_report_wait_end_timed() (for example to retry on + * EINTR) and may set it just before pgstat_report_wait_start_timed(), and a + * hook should not have to care. + */ +pg_noinline pg_attribute_cold void +pgstat_wait_event_hook_begin_slow(uint32 wait_event_info) +{ + int save_errno = errno; + wait_event_hook_type hook = wait_event_begin_hook; + + if (wait_event_hook_depth == 0) + { + wait_event_hook_depth = 1; + hook(wait_event_info); + wait_event_hook_depth = 0; + } + + errno = save_errno; +} + +pg_noinline pg_attribute_cold void +pgstat_wait_event_hook_end_slow(void) +{ + int save_errno = errno; + wait_event_hook_type hook = wait_event_end_hook; + + if (wait_event_hook_depth == 0) + { + uint32 wait_event_info; + + wait_event_info = *(volatile uint32 *) my_wait_event_info; + wait_event_hook_depth = 1; + hook(wait_event_info); + wait_event_hook_depth = 0; + } + + errno = save_errno; +} #define WAIT_EVENT_CLASS_MASK 0xFF000000 #define WAIT_EVENT_ID_MASK 0x0000FFFF diff --git a/src/include/utils/wait_event.h b/src/include/utils/wait_event.h index 86ee348220d..93184237aee 100644 --- a/src/include/utils/wait_event.h +++ b/src/include/utils/wait_event.h @@ -17,11 +17,54 @@ extern const char *pgstat_get_wait_event(uint32 wait_event_info); extern const char *pgstat_get_wait_event_type(uint32 wait_event_info); static inline void pgstat_report_wait_start(uint32 wait_event_info); static inline void pgstat_report_wait_end(void); +static inline void pgstat_report_wait_start_timed(uint32 wait_event_info); +static inline void pgstat_report_wait_end_timed(void); extern void pgstat_set_wait_event_storage(uint32 *wait_event_info); extern void pgstat_reset_wait_event_storage(void); -extern PGDLLIMPORT uint32 *my_wait_event_info; +/* + * Hooks for explicitly instrumented waits. Hook implementations may use only + * state allocated before the hook runs and written only by this process + * (backend-local memory, or this backend's own preallocated area of shared + * memory); they must not wait, allocate memory, acquire locks, or report + * errors. The depth guard prevents re-entry. + * errno is preserved for the caller by pgstat_report_wait_start_timed() and + * pgstat_report_wait_end_timed(), so hooks need not save and restore it. + * + * Hook users that chain callbacks must save the previous hook pointers, call + * the previous begin hook before their own begin work, and perform their own + * end work before calling the previous end hook. + */ +typedef void (*wait_event_hook_type) (uint32 wait_event_info); + +/* + * The wait event reporting pointer and the hook pointers live in one struct, + * so that the compiler can address all of them from one base register. + * info must stay the first member. + */ +typedef struct WaitEventReportState +{ + uint32 *info; /* where wait events are reported */ + wait_event_hook_type begin_hook; + wait_event_hook_type end_hook; +} WaitEventReportState; +extern PGDLLIMPORT WaitEventReportState wait_event_report_state; + +/* old names kept so existing code and extensions compile unchanged */ +#define my_wait_event_info (wait_event_report_state.info) +#define wait_event_begin_hook (wait_event_report_state.begin_hook) +#define wait_event_end_hook (wait_event_report_state.end_hook) + +extern PGDLLIMPORT int wait_event_hook_depth; + +/* + * Out-of-line, cold slow paths for the hook-enabled case of the timed + * wait-event pair below. See pgstat_report_wait_start_timed() for why + * these exist. + */ +extern pg_noinline pg_attribute_cold void pgstat_wait_event_hook_begin_slow(uint32 wait_event_info); +extern pg_noinline pg_attribute_cold void pgstat_wait_event_hook_end_slow(void); /* * Wait Events - Extension, InjectionPoint @@ -86,5 +129,41 @@ pgstat_report_wait_end(void) *(volatile uint32 *) my_wait_event_info = 0; } +/* + * Explicitly instrumented variant of the ordinary wait-event reporting pair. + * The ordinary functions above remain unchanged for uninstrumented sites. + * + * Every backend reaches these two functions at every instrumented wait, but + * only a process in which a consumer has installed a hook ever takes the + * enabled path. The pointer test is therefore hinted with unlikely() for the + * no-consumer path, which is laid out as the fall-through; without the hint, + * GCC's static branch predictor treats a pointer compared with NULL as + * non-NULL and lays out the enabled path there instead. The enabled path -- + * the depth guard, the volatile read in the end case, and the indirect call + * -- lives in a separate cold, noinline function rather than being inlined at + * each call site. A process with no consumer pays one predicted-not-taken + * branch per call; a process with a consumer pays one additional taken jump. + * The hook pointers are kept in wait_event_report_state next to the wait + * event pointer, so the test below addresses them from the same base; the + * old names remain as macros. The depth guard and the hook-chaining contract + * are unchanged. + */ +static inline void +pgstat_report_wait_start_timed(uint32 wait_event_info) +{ + *(volatile uint32 *) my_wait_event_info = wait_event_info; + + if (unlikely(wait_event_begin_hook != NULL)) + pgstat_wait_event_hook_begin_slow(wait_event_info); +} + +static inline void +pgstat_report_wait_end_timed(void) +{ + if (unlikely(wait_event_end_hook != NULL)) + pgstat_wait_event_hook_end_slow(); + *(volatile uint32 *) my_wait_event_info = 0; +} + #endif /* WAIT_EVENT_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b964e5ee907..3b94ff443ad 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3404,6 +3404,7 @@ WaitEventCustomEntryByInfo WaitEventCustomEntryByName WaitEventIO WaitEventIPC +WaitEventReportState WaitEventSet WaitEventTimeout WaitLSNProcInfo -- 2.49.0