8000 Improve management of SLRU statistics collection. · postgrespro/postgres_cluster@81ca868 · GitHub
[go: up one dir, main page]
Skip to content

Commit 81ca868

Browse files
tglsfdctglsfdc
committed
Improve management of SLRU statistics collection.
Instead of re-identifying which statistics bucket to use for a given SLRU on every counter increment, do it once during shmem initialization. This saves a fair number of cycles, and there's no real cost because we could not have a bucket assignment that varies over time or across backends anyway. Also, get rid of the ill-considered decision to let pgstat.c pry directly into SLRU's shared state; it's cleaner just to have slru.c pass the stats bucket number. In consequence of these changes, there's no longer any need to store an SLRU's LWLock tranche info in shared memory, so get rid of that, making this a net reduction in shmem consumption. (That partly reverts fe702a7.) This is basically code review for 28cac71, so I also cleaned up some comments, removed a dangling extern declaration, fixed some things that should be static and/or const, etc. Discussion: https://postgr.es/m/3618.1589313035@sss.pgh.pa.us
1 parent 850196b commit 81ca868

File tree

5 files changed

+76
-85
lines changed

5 files changed

+76
-85
lines changed

‎src/backend/access/transam/slru.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
191191

192192
/* shared->latest_page_number will be set later */
193193

194+
shared->slru_stats_idx = pgstat_slru_index(name);
195+
194196
ptr = (char *) shared;
195197
offset = MAXALIGN(sizeof(SlruSharedData));
196198
shared->page_buffer = (char **) (ptr + offset);
@@ -214,15 +216,11 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
214216
offset += MAXALIGN(nslots * nlsns * sizeof(XLogRecPtr));
215217
}
216218

217-
Assert(strlen(name) + 1 < SLRU_MAX_NAME_LENGTH);
218-
strlcpy(shared->lwlock_tranche_name, name, SLRU_MAX_NAME_LENGTH);
219-
shared->lwlock_tranche_id = tranche_id;
220-
221219
ptr += BUFFERALIGN(offset);
222220
for (slotno = 0; slotno < nslots; slotno++)
223221
{
224222
LWLockInitialize(&shared->buffer_locks[slotno].lock,
225-
shared->lwlock_tranche_id);
223+
tranche_id);
226224

227225
shared->page_buffer[slotno] = ptr;
228226
shared->page_status[slotno] = SLRU_PAGE_EMPTY;
@@ -238,8 +236,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
238236
Assert(found);
239237

240238
/* Register SLRU tranche in the main tranches array */
241-
LWLockRegisterTranche(shared->lwlock_tranche_id,
242-
shared->lwlock_tranche_name);
239+
LWLockRegisterTranche(tranche_id, name);
243240

244241
/*
245242
* Initialize the unshared control struct, including directory path. We
@@ -287,7 +284,7 @@ SimpleLruZeroPage(SlruCtl ctl, int pageno)
287284
shared->latest_page_number = pageno;
288285

289286
/* update the stats counter of zeroed pages */
290-
pgstat_count_slru_page_zeroed(ctl);
287+
pgstat_count_slru_page_zeroed(shared->slru_stats_idx);
291288

292289
return slotno;
293290
}
@@ -408,7 +405,7 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
408405
SlruRecentlyUsed(shared, slotno);
409406

410407
/* update the stats counter of pages found in the SLRU */
411-
pgstat_count_slru_page_hit(ctl);
408+
pgstat_count_slru_page_hit(shared->slru_stats_idx);
412409

413410
return slotno;
414411
}
@@ -453,7 +450,7 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
453450
SlruRecentlyUsed(shared, slotno);
454451

455452
/* update the stats counter of pages not found in SLRU */
456-
pgstat_count_slru_page_read(ctl);
453+
pgstat_count_slru_page_read(shared->slru_stats_idx);
457454

458455
return slotno;
459456
}
@@ -493,7 +490,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
493490
SlruRecentlyUsed(shared, slotno);
494491

495492
/* update the stats counter of pages found in the SLRU */
496-
pgstat_count_slru_page_hit(ctl);
493+
pgstat_count_slru_page_hit(shared->slru_stats_idx);
497494

498495
return slotno;
499496
}
@@ -612,7 +609,7 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno)
612609
off_t endpos;
613610

614611
/* update the stats counter of checked pages */
615-
pgstat_count_slru_page_exists(ctl);
612+
pgstat_count_slru_page_exists(ctl->shared->slru_stats_idx);
616613

617614
SlruFileName(ctl, path, segno);
618615

@@ -749,7 +746,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
749746
int fd = -1;
750747

751748
/* update the stats counter of written pages */
752-
pgstat_count_slru_page_written(ctl);
749+
pgstat_count_slru_page_written(shared->slru_stats_idx);
753750

754751
/*
755752
* Honor the write-WAL-before-data rule, if appropriate, so that we do not
@@ -1147,7 +1144,7 @@ SimpleLruFlush(SlruCtl ctl, bool allow_redirtied)
11471144
bool ok;
11481145

11491146
/* update the stats counter of flushes */
1150-
pgstat_count_slru_flush(ctl);
1147+
pgstat_count_slru_flush(shared->slru_stats_idx);
11511148

11521149
/*
11531150
* Find and write dirty pages
@@ -1211,7 +1208,7 @@ SimpleLruTruncate(SlruCtl ctl, int cutoffPage)
12111208
int slotno;
12121209

12131210
/* update the stats counter of truncates */
1214-
pgstat_count_slru_truncate(ctl);
1211+
pgstat_count_slru_truncate(shared->slru_stats_idx);
12151212

12161213
/*
12171214
* The cutoff point is the start of the segment containing cutoffPage.

‎src/backend/postmaster/pgstat.c

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -142,24 +142,30 @@ char *pgstat_stat_tmpname = NULL;
142142
PgStat_MsgBgWriter BgWriterStats;
143143

144144
/*
145-
* SLRU statistics counters (unused in other processes) stored directly in
146-
* stats structure so it can be sent without needing to copy things around.
147-
* We assume this inits to zeroes. There is no central registry of SLRUs,
148-
* so we use this fixed list instead.
149-
*
150-
* There's a separte entry for each SLRU we have. The "other" entry is used
151-
* for all SLRUs without an explicit entry (e.g. SLRUs in extensions).
152-
*/
153-
static char *slru_names[] = {"async", "clog", "commit_timestamp",
154-
"multixact_offset", "multixact_member",
155-
"oldserxid", "subtrans",
156-
"other" /* has to be last */};
145+
* List of SLRU names that we keep stats for. There is no central registry of
146+
* SLRUs, so we use this fixed list instead. The "other" entry is used for
147+
* all SLRUs without an explicit entry (e.g. SLRUs in extensions).
148+
*/
149+
static const char *const slru_names[] = {
150+
"async",
151+
"clog",
152+
"commit_timestamp",
153+
"multixact_offset",
154+
"multixact_member",
155+
"oldserxid",
156+
"subtrans",
157+
"other" /* has to be last */
158+
};
159+
160+
#define SLRU_NUM_ELEMENTS lengthof(slru_names)
157161

158-
/* number of elemenents of slru_name array */
159-
#define SLRU_NUM_ELEMENTS (sizeof(slru_names) / sizeof(char *))
160-
161-
/* entries in the same order as slru_names */
162-
PgStat_MsgSLRU SLRUStats[SLRU_NUM_ELEMENTS];
162+
/*
163+
* SLRU statistics counts waiting to be sent to the collector. These are
164+
* stored directly in stats message format so they can be sent without needing
165+
* to copy things around. We assume this variable inits to zeroes. Entries
166+
* are one-to-one with slru_names[].
167+
*/
168+
static PgStat_MsgSLRU SLRUStats[SLRU_NUM_ELEMENTS];
163169

164170
/* ----------
165171
* Local data
@@ -4411,12 +4417,10 @@ pgstat_send_bgwriter(void)
44114417
static void
44124418
pgstat_send_slru(void)
44134419
{
4414-
int i;
4415-
44164420
/* We assume this initializes to zeroes */
44174421
static const PgStat_MsgSLRU all_zeroes;
44184422

4419-
for (i = 0; i < SLRU_NUM_ELEMENTS; i++)
4423+
for (int i = 0; i < SLRU_NUM_ELEMENTS; i++)
44204424
{
44214425
/*
44224426
* This function can be called even if nothing at all has happened. In
@@ -6705,15 +6709,13 @@ pgstat_slru_index(const char *name)
67056709
* in which case this returns NULL. This allows writing code that does not
67066710
* know the number of entries in advance.
67076711
*/
6708-
char *
6709-
pgstat_slru_name(int idx)
6712+
const char *
6713+
pgstat_slru_name(int slru_idx)
67106714
{
6711-
Assert(idx >= 0);
6712-
6713-
if (idx >= SLRU_NUM_ELEMENTS)
6715+
if (slru_idx < 0 || slru_idx >= SLRU_NUM_ELEMENTS)
67146716
return NULL;
67156717

6716-
return slru_names[idx];
6718+
return slru_names[slru_idx];
67176719
}
67186720

67196721
/*
@@ -6722,54 +6724,56 @@ pgstat_slru_name(int idx)
67226724
* Returns pointer to entry with counters for given SLRU (based on the name
67236725
* stored in SlruCtl as lwlock tranche name).
67246726
*/
6725-
static PgStat_MsgSLRU *
6726-
slru_entry(SlruCtl ctl)
6727+
static inline PgStat_MsgSLRU *
6728+
slru_entry(int slru_idx)
67276729
{
6728-
int idx = pgstat_slru_index(ctl->shared->lwlock_tranche_name);
6729-
6730-
Assert((idx >= 0) && (idx < SLRU_NUM_ELEMENTS));
6730+
Assert((slru_idx >= 0) && (slru_idx < SLRU_NUM_ELEMENTS));
67316731

6732-
return &SLRUStats[idx];
6732+
return &SLRUStats[slru_idx];
67336733
}
67346734

6735+
/*
6736+
* SLRU statistics count accumulation functions --- called from slru.c
6737+
*/
6738+
67356739
void
6736-
pgstat_count_slru_page_zeroed(SlruCtl ctl)
6740+
pgstat_count_slru_page_zeroed(int slru_idx)
67376741
{
6738-
slru_entry(ctl)->m_blocks_zeroed 1241 += 1;
6742+
slru_entry(slru_idx)->m_blocks_zeroed += 1;
67396743
}
67406744

67416745
void
6742-
pgstat_count_slru_page_hit(SlruCtl ctl)
6746+
pgstat_count_slru_page_hit(int slru_idx)
67436747
{
6744-
slru_entry(ctl)->m_blocks_hit += 1;
6748+
slru_entry(slru_idx)->m_blocks_hit += 1;
67456749
}
67466750

67476751
void
6748-
pgstat_count_slru_page_exists(SlruCtl ctl)
6752+
pgstat_count_slru_page_exists(int slru_idx)
67496753
{
6750-
slru_entry(ctl)->m_blocks_exists += 1;
6754+
slru_entry(slru_idx)->m_blocks_exists += 1;
67516755
}
67526756

67536757
void
6754-
pgstat_count_slru_page_read(SlruCtl ctl)
6758+
pgstat_count_slru_page_read(int slru_idx)
67556759
{
6756-
slru_entry(ctl)->m_blocks_read += 1;
6760+
slru_entry(slru_idx)->m_blocks_read += 1;
67576761
}
67586762

67596763
void
6760-
pgstat_count_slru_page_written(SlruCtl ctl)
6764+
pgstat_count_slru_page_written(int slru_idx)
67616765
{
6762-
slru_entry(ctl)->m_blocks_written += 1;
6766+
slru_entry(slru_idx)->m_blocks_written += 1;
67636767
}
67646768

67656769
void
6766-
pgstat_count_slru_flush(SlruCtl ctl)
6770+
pgstat_count_slru_flush(int slru_idx)
67676771
{
6768-
slru_entry(ctl)->m_flush += 1;
6772+
slru_entry(slru_idx)->m_flush += 1;
67696773
}
67706774

67716775
void
6772-
pgstat_count_slru_truncate(SlruCtl ctl)
6776+
pgstat_count_slru_truncate(int slru_idx)
67736777
{
6774-
slru_entry(ctl)->m_truncate += 1;
6778+
slru_entry(slru_idx)->m_truncate += 1;
67756779
}

‎src/backend/utils/adt/pgstatfuncs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1739,7 +1739,7 @@ pg_stat_get_slru(PG_FUNCTION_ARGS)
17391739
Datum values[PG_STAT_GET_SLRU_COLS];
17401740
bool nulls[PG_STAT_GET_SLRU_COLS];
17411741
PgStat_SLRUStats stat = stats[i];
1742-
char *name;
1742+
const char *name;
17431743

17441744
name = pgstat_slru_name(i);
17451745

‎src/include/access/slru.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@
3232
*/
3333
#define SLRU_PAGES_PER_SEGMENT 32
3434

35-
/* Maximum length of an SLRU name */
36-
#define SLRU_MAX_NAME_LENGTH 32
37-
3835
/*
3936
* Page status codes. Note that these do not include the "dirty" bit.
4037
* page_dirty can be true only in the VALID or WRITE_IN_PROGRESS states;
@@ -68,6 +65,7 @@ typedef struct SlruSharedData
6865
bool *page_dirty;
6966
int *page_number;
7067
int *page_lru_count;
68+
LWLockPadded *buffer_locks;
7169

7270
/*
7371
* Optional array of WAL flush LSNs associated with entries in the SLRU
@@ -98,10 +96,8 @@ typedef struct SlruSharedData
9896
*/
9997
int latest_page_number;
10098

101-
/* LWLocks */
102-
int lwlock_tranche_id;
103-
char lwlock_tranche_name[SLRU_MAX_NAME_LENGTH];
104-
LWLockPadded *buffer_locks;
99+
/* SLRU's index for statistics purposes (might not be unique) */
100+
int slru_stats_idx;
105101
} SlruSharedData;
106102

107103
typedef SlruSharedData *SlruShared;

‎src/include/pgstat.h

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#ifndef PGSTAT_H
1212
#define PGSTAT_H
1313

14-
#include "access/slru.h"
1514
#include "datatype/timestamp.h"
1615
#include "libpq/pqcomm.h"
1716
#include "miscadmin.h"
@@ -438,7 +437,7 @@ typedef struct PgStat_MsgBgWriter
438437
} PgStat_MsgBgWriter;
439438

440439
/* ----------
441-
* PgStat_MsgSLRU Sent by the SLRU to update statistics.
440+
* PgStat_MsgSLRU Sent by a backend to update SLRU statistics.
442441
* ----------
443442
*/
444443
typedef struct PgStat_MsgSLRU
@@ -1260,11 +1259,6 @@ extern char *pgstat_stat_filename;
12601259
*/
12611260
extern PgStat_MsgBgWriter BgWriterStats;
12621261

1263-
/*
1264-
* SLRU statistics counters are updated directly by slru.
1265-
*/
1266-
extern PgStat_MsgSLRU SlruStats[];
1267-
12681262
/*
12691263
* Updated by pgstat_count_buffer_*_time macros
12701264
*/
@@ -1480,14 +1474,14 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
14801474
extern PgStat_GlobalStats *pgstat_fetch_global(void);
14811475
extern PgStat_SLRUStats *pgstat_fetch_slru(void);
14821476

1483-
extern void pgstat_count_slru_page_zeroed(SlruCtl ctl);
1484-
extern void pgstat_count_slru_page_hit(SlruCtl ctl);
1485-
extern void pgstat_count_slru_page_read(SlruCtl ctl);
1486-
extern void pgstat_count_slru_page_written(SlruCtl ctl);
1487-
extern void pgstat_count_slru_page_exists(SlruCtl ctl);
1488-
extern void pgstat_count_slru_flush(SlruCtl ctl);
1489-
extern void pgstat_count_slru_truncate(SlruCtl ctl);
1490-
extern char *pgstat_slru_name(int idx);
1491-
extern int pgstat_slru_index(const char *name);
1477+
extern void pgstat_count_slru_page_zeroed(int slru_idx);
1478+
extern void pgstat_count_slru_page_hit(int slru_idx);
1479+
extern void pgstat_count_slru_page_read(int slru_idx);
1480+
extern void pgstat_count_slru_page_written(int slru_idx);
1481+
extern void pgstat_count_slru_page_exists(int slru_idx);
1482+
extern void pgstat_count_slru_flush(int slru_idx);
1483+
extern void pgstat_count_slru_truncate(int slru_idx);
1484+
extern const char *pgstat_slru_name(int slru_idx);
1485+
extern int pgstat_slru_index(const char *name);
14921486

14931487
#endif /* PGSTAT_H */

0 commit comments

Comments
 (0)
0