From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Inefficiency in SLRU stats collection |
Date: | 2020-05-12 19:50:35 |
Message-ID: | 3618.1589313035@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I happened to come across this code added by 28cac71bd:
static PgStat_MsgSLRU *
slru_entry(SlruCtl ctl)
{
int idx = pgstat_slru_index(ctl->shared->lwlock_tranche_name);
Assert((idx >= 0) && (idx < SLRU_NUM_ELEMENTS));
return &SLRUStats[idx];
}
which is invoked by all the pgstat_count_slru_XXX routines.
This seems mightily inefficient --- the count functions are
just there to increment integer counters, but first they
have to do up to half a dozen strcmp's to figure out which
counter to increment.
We could improve this by adding another integer field to
SlruSharedData (which is already big enough that no one
would notice) and recording the result of pgstat_slru_index()
there as soon as the lwlock_tranche_name is set. (In fact,
it looks like we could stop saving the tranche name as such
altogether, thus buying back way more shmem than the integer
field would occupy.)
This does require the assumption that all backends agree
on the SLRU stats index for a particular SLRU cache. But
AFAICS we're locked into that already, since the backends
use those indexes to tell the stats collector which cache
they're sending stats for.
Thoughts?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Laurenz Albe | 2020-05-12 19:50:40 | Re: COPY, lock release and MVCC |
Previous Message | Simon Riggs | 2020-05-12 19:30:20 | Re: Our naming of wait events is a disaster. |