Re: [Proposal] Add accumulated statistics for wait event
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Cc: Daniel Gustafsson <daniel@yesql.se>, imai.yoshikazu@fujitsu.com, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "yoshikazu_i443@live.jp" <yoshikazu_i443@live.jp>
Date: 2021-06-14T22:01:14Z
Lists: pgsql-hackers
Hi, On 2021-06-14 23:20:47 +0200, Jehan-Guillaume de Rorthais wrote: > > On 2021-06-14 16:10:32 +0200, Jehan-Guillaume de Rorthais wrote: > > > In the patch in attachment, I tried to fix this by using kind of an internal > > > hook for pgstat_report_wait_start and pgstat_report_wait_end. This allows to > > > "instrument" wait events only when required, on the fly, dynamically. > > > > That's *far worse*. You're adding an indirect function call. Which requires > > loading a global variable and then a far call to a different function. You're > > changing a path that's ~2 instructions with minimal dependencies (and no > > branches (i.e. fully out of order executable) to something on the order of ~15 > > instructions with plenty dependencies and at least two branches (call, ret). > > Oh, I didn't realized it would affect all queries, even when log_statement_stats > was off. Thank you for your explanation. Maybe I just am misunderstanding what you were doing? As far as I can tell your patch changed pgstat_report_wait_start() to be an indirect function call - right? Then yes, this adds overhead to everything. You *could* add a pgstat_report_wait_(start|end)_with_time() or such and only use that in places that won't have a high frequency. But I just don't quite see the use-case for that. Greetings, Andres Freund