public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Maciek Sakrejda <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Lukas Fittl <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Thomas Munro <[email protected]>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: Tue, 29 Nov 2022 20:51:13 -0600
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_aioBQpF19Jkh75q7gid+FaW-JnFoGw9b-zeyhqD4w2+w@mail.gmail.com>
References: <CAAKRu_a_7JDexgYGFGeNiFfjBBkhwcPwjnkesNnTuF6hFgUY5A@mail.gmail.com>
<CAOtHd0Aj-F1ogXiEWE4wV5U8A8a-mxS=hYwx9B3fsg57hG2zWg@mail.gmail.com>
<CAAKRu_agmNMyMbfffGegbgFpUUTL=k3U64n_v0gLQGW5DeDkhg@mail.gmail.com>
<[email protected]>
<CAOtHd0BkyGEXN6odK71=PZzK_TndbZkFstwgrtX-vA39KOpbuA@mail.gmail.com>
<CAAKRu_YKUGKYCZrXCgtky5jx04QhvGMdkNDEN0uhBWPuVkvOpQ@mail.gmail.com>
<CAOtHd0DU9Ymo+XQ_2Ddcacr2wS_a8-B4+AUVzJbSkDi8jkd4Xw@mail.gmail.com>
<CAAKRu_bb8GupZOLBasf2XkHBrkj8ow8=ZOmcBOj4_G90h2TFKQ@mail.gmail.com>
<[email protected]>
<CAAKRu_aioBQpF19Jkh75q7gid+FaW-JnFoGw9b-zeyhqD4w2+w@mail.gmail.com>
On Mon, Nov 28, 2022 at 09:08:36PM -0500, Melanie Plageman wrote:
> > +pgstat_io_op_stats_collected(BackendType bktype)
> > +{
> > + return bktype != B_INVALID && bktype != B_ARCHIVER && bktype != B_LOGGER &&
> > + bktype != B_WAL_RECEIVER && bktype != B_WAL_WRITER;
> >
> > Similar: I'd prefer to see this as 5 "ifs" or a "switch" to return
> > false, else return true. But YMMV.
>
> I don't know that separating it into multiple if statements or a switch
> would make it more clear to me or help me with debugging here.
>
> Separately, since this is used in non-assert builds, I would like to
> ensure it is efficient. Do you know if a switch or if statements will
> be compiled to the exact same thing as this at useful optimization
> levels?
This doesn't seem like a detail worth much bother, but I did a test.
With -O2 (but not -O1 nor -Og) the assembly (gcc 9.4) is the same when
written like:
+ if (bktype == B_INVALID)
+ return false;
+ if (bktype == B_ARCHIVER)
+ return false;
+ if (bktype == B_LOGGER)
+ return false;
+ if (bktype == B_WAL_RECEIVER)
+ return false;
+ if (bktype == B_WAL_WRITER)
+ return false;
+
+ return true;
objdump --disassemble=pgstat_io_op_stats_collected src/backend/postgres_lib.a.p/utils_activity_pgstat_io_ops.c.o
0000000000000110 <pgstat_io_op_stats_collected>:
110: f3 0f 1e fa endbr64
114: b8 01 00 00 00 mov $0x1,%eax
119: 83 ff 0d cmp $0xd,%edi
11c: 77 10 ja 12e <pgstat_io_op_stats_collected+0x1e>
11e: b8 03 29 00 00 mov $0x2903,%eax
123: 89 f9 mov %edi,%ecx
125: 48 d3 e8 shr %cl,%rax
128: 48 f7 d0 not %rax
12b: 83 e0 01 and $0x1,%eax
12e: c3 retq
I was surprised, but the assembly is *not* the same when I used a switch{}.
I think it's fine to write however you want.
> > pgstat_count_io_op() has a superflous newline before "}".
>
> I couldn't find the one you are referencing.
> Do you mind pasting in the code?
+ case IOOP_WRITE:
+ pending_counters->writes++;
+ break;
+ }
+ --> here <--
+}
--
Justin
view thread (108+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox