| From: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pg_stat_io_histogram |
| Date: | 2026-02-11 10:43:23 |
| Message-ID: | CAKZiRmws9w2-sunYM7eq30FF-t=LjQBy7WBQOh94Lxmn4zX21w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Feb 5, 2026 at 1:13 PM Jakub Wartak
<jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
>
> On Fri, Jan 30, 2026 at 2:43 PM Jakub Wartak
> <jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
> [..]
> > I'm attaching v3 which has now default switched to __builtin_clzl() which
> > works ok for uint64 (not sure if I need to care about __builtin_clzll
> > on Windows?).
>
> Here comes the v4:
>
> 1. Rebased just in case.
> 2. Earlier appears to the uncomplete patch without local changes (comment
> mentioned use of __builtin_clzl, but actually code called
> __builtin_clz -- 32-bit one not long one), fixed that with the new
> version.
> 3. I've added discovery of __builtin_clzl into autoconf/meson as it was
> missing (although comment there says "We assume that we needn't test
> all widths of these explicitly:", but isn't it safer we test explicitly
> what we use?
> 4. And then I've spotted that pg_leftmost_one_pos64() in pg_binutils.h uses on
> master the __builtin_clzl already, so I've tweaked it to use check
> HAVE__BUILTIN_CLZL (not CLZ) too once we have that now.
>
> Open questions:
> 0. Should I pursue more benchmarking or the above results are enough?
> 1. Should I add per-PID backend stats too or skip that to avoid causing
> potential further overhead? (probably yet another memcpy...)
> 2. Shouldn't we fix that mdsyncfiletag() mentioned earlier we seem to have
> pgstat_count_io_op_time() *after* potential FileClose() (as per my
> earlier question)
Hi all,
I would be grateful for any feedback. Here comes v5 attached with:
1. finish documentation for I/O operation types (io_type column) and s/hit?/hit/
2. fix documentation typos, references, copy paste errors..
3. fix code comments typos
4. added missing return for pg_leading_zero_bits64() in case of lack
of HAVE__BUILTIN_CLZL
(discovered thanks to clang's -Wreturn-type)
There are still two implementations inside for get_bucket_index()
I think we should stick to the clz one as it appears to be faster.
Above questions since v4 remain.
-J.
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-Add-pg_stat_io_histogram-view-to-provide-more-det.patch | text/x-patch | 35.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-02-11 10:58:41 | Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals) |
| Previous Message | Aditya Kamath | 2026-02-11 10:41:09 | RE: Decoupling our alignment assumptions about int64 and double |