database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-08-28 18:48:04 +03:00
Code Activity

Improve efficiency of wait event reporting, remove proc.h dependency.

Browse Source 
pgstat_report_wait_start() and pgstat_report_wait_end() required two
conditional branches so far. One to check if MyProc is NULL, the other to
check if pgstat_track_activities is set. As wait events are used around
comparatively lightweight operations, and are inlined (reducing branch
predictor effectiveness), that's not great.

The dependency on MyProc has a second disadvantage: Low-level subsystems, like
storage/file/fd.c, report wait events, but architecturally it is preferable
for them to not depend on inter-process subsystems like proc.h (defining
PGPROC).  After this change including pgstat.h (nor obviously its
sub-components like backend_status.h, wait_event.h, ...) does not pull in IPC
related headers anymore.

These goals, efficiency and abstraction, are achieved by having
pgstat_report_wait_start/end() not interact with MyProc, but instead a new
my_wait_event_info variable. At backend startup it points to a local variable,
removing the need to check for MyProc being NULL. During process
initialization my_wait_event_info is redirected to MyProc->wait_event_info. At
shutdown this is reversed. Because wait event reporting now does not need to
know about where the wait event is stored, it does not need to know about
PGPROC anymore.

The removal of the branch for checking pgstat_track_activities is simpler:
Don't check anymore. The cost due to the branch are often higher than the
store - and even if not, pgstat_track_activities is rarely disabled.

The main motivator to commit this work now is that removing the (indirect)
pgproc.h include from pgstat.h simplifies a patch to move statistics reporting
to shared memory (which still has a chance to get into 14).

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210402194458.2vu324hkk2djq6ce@alap3.anarazel.de
This commit is contained in:
Andres Freund
2021-04-03 11:44:47 -07:00
parent e1025044cd
commit 225a22b19e
3 changed files with 69 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/backend/storage/lmgr/proc.c
View File

@@ -448,6 +448,9 @@ InitProcess(void)
OwnLatch(&MyProc->procLatch);
SwitchToSharedLatch();
/* now that we have a proc, report wait events to shared memory */
pgstat_set_wait_event_storage(&MyProc->wait_event_info);
/*
* We might be reusing a semaphore that belonged to a failed process. So
* be careful and reinitialize its value here. (This is not strictly
@@ -601,6 +604,9 @@ InitAuxiliaryProcess(void)
OwnLatch(&MyProc->procLatch);
SwitchToSharedLatch();
/* now that we have a proc, report wait events to shared memory */
pgstat_set_wait_event_storage(&MyProc->wait_event_info);
/* Check that group locking fields are in a proper initial state. */
Assert(MyProc->lockGroupLeader == NULL);
Assert(dlist_is_empty(&MyProc->lockGroupMembers));
@@ -884,10 +890,15 @@ ProcKill(int code, Datum arg)
/*
* Reset MyLatch to the process local one. This is so that signal
* handlers et al can continue using the latch after the shared latch
* isn't ours anymore. After that clear MyProc and disown the shared
* latch.
* isn't ours anymore.
*
* Similarly, stop reporting wait events to MyProc->wait_event_info.
*
* After that clear MyProc and disown the shared latch.
*/
SwitchBackToLocalLatch();
pgstat_reset_wait_event_storage();
proc = MyProc;
MyProc = NULL;
DisownLatch(&proc->procLatch);
@@ -952,13 +963,10 @@ AuxiliaryProcKill(int code, Datum arg)
/* Cancel any pending condition variable sleep, too */
ConditionVariableCancelSleep();
/*
* Reset MyLatch to the process local one. This is so that signal
* handlers et al can continue using the latch after the shared latch
* isn't ours anymore. After that clear MyProc and disown the shared
* latch.
*/
/* look at the equivalent ProcKill() code for comments */
SwitchBackToLocalLatch();
pgstat_reset_wait_event_storage();
proc = MyProc;
MyProc = NULL;
DisownLatch(&proc->procLatch);