Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit225a22b

Browse files
committed
Improve efficiency of wait event reporting, remove proc.h dependency.
pgstat_report_wait_start() and pgstat_report_wait_end() required twoconditional branches so far. One to check if MyProc is NULL, the other tocheck if pgstat_track_activities is set. As wait events are used aroundcomparatively lightweight operations, and are inlined (reducing branchpredictor effectiveness), that's not great.The dependency on MyProc has a second disadvantage: Low-level subsystems, likestorage/file/fd.c, report wait events, but architecturally it is preferablefor them to not depend on inter-process subsystems like proc.h (definingPGPROC). After this change including pgstat.h (nor obviously itssub-components like backend_status.h, wait_event.h, ...) does not pull in IPCrelated headers anymore.These goals, efficiency and abstraction, are achieved by havingpgstat_report_wait_start/end() not interact with MyProc, but instead a newmy_wait_event_info variable. At backend startup it points to a local variable,removing the need to check for MyProc being NULL. During processinitialization my_wait_event_info is redirected to MyProc->wait_event_info. Atshutdown this is reversed. Because wait event reporting now does not need toknow about where the wait event is stored, it does not need to know aboutPGPROC 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 thestore - 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 reportingto 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
1 parente102504 commit225a22b

File tree

3 files changed

+69
-38
lines changed

3 files changed

+69
-38
lines changed

‎src/backend/storage/lmgr/proc.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,9 @@ InitProcess(void)
448448
OwnLatch(&MyProc->procLatch);
449449
SwitchToSharedLatch();
450450

451+
/* now that we have a proc, report wait events to shared memory */
452+
pgstat_set_wait_event_storage(&MyProc->wait_event_info);
453+
451454
/*
452455
* We might be reusing a semaphore that belonged to a failed process. So
453456
* be careful and reinitialize its value here. (This is not strictly
@@ -601,6 +604,9 @@ InitAuxiliaryProcess(void)
601604
OwnLatch(&MyProc->procLatch);
602605
SwitchToSharedLatch();
603606

607+
/* now that we have a proc, report wait events to shared memory */
608+
pgstat_set_wait_event_storage(&MyProc->wait_event_info);
609+
604610
/* Check that group locking fields are in a proper initial state. */
605611
Assert(MyProc->lockGroupLeader==NULL);
606612
Assert(dlist_is_empty(&MyProc->lockGroupMembers));
@@ -884,10 +890,15 @@ ProcKill(int code, Datum arg)
884890
/*
885891
* Reset MyLatch to the process local one. This is so that signal
886892
* handlers et al can continue using the latch after the shared latch
887-
* isn't ours anymore. After that clear MyProc and disown the shared
888-
* latch.
893+
* isn't ours anymore.
894+
*
895+
* Similarly, stop reporting wait events to MyProc->wait_event_info.
896+
*
897+
* After that clear MyProc and disown the shared latch.
889898
*/
890899
SwitchBackToLocalLatch();
900+
pgstat_reset_wait_event_storage();
901+
891902
proc=MyProc;
892903
MyProc=NULL;
893904
DisownLatch(&proc->procLatch);
@@ -952,13 +963,10 @@ AuxiliaryProcKill(int code, Datum arg)
952963
/* Cancel any pending condition variable sleep, too */
953964
ConditionVariableCancelSleep();
954965

955-
/*
956-
* Reset MyLatch to the process local one. This is so that signal
957-
* handlers et al can continue using the latch after the shared latch
958-
* isn't ours anymore. After that clear MyProc and disown the shared
959-
* latch.
960-
*/
966+
/* look at the equivalent ProcKill() code for comments */
961967
SwitchBackToLocalLatch();
968+
pgstat_reset_wait_event_storage();
969+
962970
proc=MyProc;
963971
MyProc=NULL;
964972
DisownLatch(&proc->procLatch);

‎src/backend/utils/activity/wait_event.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@
77
*
88
* IDENTIFICATION
99
* src/backend/postmaster/wait_event.c
10+
*
11+
* NOTES
12+
*
13+
* To make pgstat_report_wait_start() and pgstat_report_wait_end() as
14+
* lightweight as possible, they do not check if shared memory (MyProc
15+
* specifically, where the wait event is stored) is already available. Instead
16+
* we initially set my_wait_event_info to a process local variable, which then
17+
* is redirected to shared memory using pgstat_set_wait_event_storage(). For
18+
* the same reason pgstat_track_activities is not checked - the check adds
19+
* more work than it saves.
20+
*
1021
* ----------
1122
*/
1223
#include"postgres.h"
@@ -23,6 +34,36 @@ static const char *pgstat_get_wait_timeout(WaitEventTimeout w);
2334
staticconstchar*pgstat_get_wait_io(WaitEventIOw);
2435

2536

37+
staticuint32local_my_wait_event_info;
38+
uint32*my_wait_event_info=&local_my_wait_event_info;
39+
40+
41+
/*
42+
* Configure wait event reporting to report wait events to *wait_event_info.
43+
* *wait_event_info needs to be valid until pgstat_reset_wait_event_storage()
44+
* is called.
45+
*
46+
* Expected to be called during backend startup, to point my_wait_event_info
47+
* into shared memory.
48+
*/
49+
void
50+
pgstat_set_wait_event_storage(uint32*wait_event_info)
51+
{
52+
my_wait_event_info=wait_event_info;
53+
}
54+
55+
/*
56+
* Reset wait event storage location.
57+
*
58+
* Expected to be called during backend shutdown, before the location set up
59+
* pgstat_set_wait_event_storage() becomes invalid.
60+
*/
61+
void
62+
pgstat_reset_wait_event_storage(void)
63+
{
64+
my_wait_event_info=&local_my_wait_event_info;
65+
}
66+
2667
/* ----------
2768
* pgstat_get_wait_event_type() -
2869
*

‎src/include/utils/wait_event.h

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@
1111
#defineWAIT_EVENT_H
1212

1313

14-
#include"storage/proc.h"/* for MyProc */
15-
16-
1714
/* ----------
1815
* Wait Classes
1916
* ----------
@@ -234,13 +231,10 @@ extern const char *pgstat_get_wait_event(uint32 wait_event_info);
234231
externconstchar*pgstat_get_wait_event_type(uint32wait_event_info);
235232
staticinlinevoidpgstat_report_wait_start(uint32wait_event_info);
236233
staticinlinevoidpgstat_report_wait_end(void);
234+
externvoidpgstat_set_wait_event_storage(uint32*wait_event_info);
235+
externvoidpgstat_reset_wait_event_storage(void);
237236

238-
239-
/*
240-
* Repeated here for the inline functions because it is declared in pgstat.h,
241-
* which includes this header.
242-
*/
243-
externPGDLLIMPORTboolpgstat_track_activities;
237+
externPGDLLIMPORTuint32*my_wait_event_info;
244238

245239

246240
/* ----------
@@ -254,47 +248,35 @@ extern PGDLLIMPORT bool pgstat_track_activities;
254248
*for wait event which is sufficient for current usage, 1-byte is
255249
*reserved for future usage.
256250
*
257-
* NB: this *must* be able to survive being called before MyProc has been
258-
* initialized.
251+
*Historically we used to make this reporting conditional on
252+
*pgstat_track_activities, but the check for that seems to add more cost
253+
*than it saves.
254+
*
255+
*my_wait_event_info initially points to local memory, making it safe to
256+
*call this before MyProc has been initialized.
259257
* ----------
260258
*/
261259
staticinlinevoid
262260
pgstat_report_wait_start(uint32wait_event_info)
263261
{
264-
volatilePGPROC*proc=MyProc;
265-
266-
if (!pgstat_track_activities|| !proc)
267-
return;
268-
269262
/*
270263
* Since this is a four-byte field which is always read and written as
271264
* four-bytes, updates are atomic.
272265
*/
273-
proc->wait_event_info=wait_event_info;
266+
*(volatileuint32*)my_wait_event_info=wait_event_info;
274267
}
275268

276269
/* ----------
277270
* pgstat_report_wait_end() -
278271
*
279272
*Called to report end of a wait.
280-
*
281-
* NB: this *must* be able to survive being called before MyProc has been
282-
* initialized.
283273
* ----------
284274
*/
285275
staticinlinevoid
286276
pgstat_report_wait_end(void)
287277
{
288-
volatilePGPROC*proc=MyProc;
289-
290-
if (!pgstat_track_activities|| !proc)
291-
return;
292-
293-
/*
294-
* Since this is a four-byte field which is always read and written as
295-
* four-bytes, updates are atomic.
296-
*/
297-
proc->wait_event_info=0;
278+
/* see pgstat_report_wait_start() */
279+
*(volatileuint32*)my_wait_event_info=0;
298280
}
299281

300282

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp