pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: David Geier <geidav.pg@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Lukas Fittl <lukas@fittl.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Cc: Maciek Sakrejda <m.sakrejda@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?
Date: Thu, 26 Jan 2023 12:21:13 +0100
Message-ID: <3ac157f7-085d-e071-45fc-b87cd306360c@gmail.com> (raw)
In-Reply-To: <20230123203006.qgqhtxh447qoj4yr@awork3.anarazel.de>
References: <b201ac3c-1bff-2414-be8e-fc287f78be1a@gmail.com>
	<20230113195547.k4nlrmawpijqwlsa@awork3.anarazel.de>
	<CA+TgmoYOvh=k-H9m21Lh-SWbn7TNurm3JoOVxW+kOO=Gn1_8Xw@mail.gmail.com>
	<20230117164758.gx4uuzhk5grw7zea@awork3.anarazel.de>
	<3044621.1673976417@sss.pgh.pa.us>
	<20230117185053.owqxw4ebp5ny6zhd@awork3.anarazel.de>
	<51876d9a-6812-24b6-57ab-a555e85afb6b@gmail.com>
	<20230121041613.xhjfzk7iobhpqm3u@awork3.anarazel.de>
	<20230121053157.hsdemsvq7oqydfl6@awork3.anarazel.de>
	<7c5f2170-f99d-5cd2-cb80-375155107ca7@gmail.com>
	<20230123203006.qgqhtxh447qoj4yr@awork3.anarazel.de>

Hi,

On 1/23/23 21:30, Andres Freund wrote:
> That's been the case since my first post in the thread :). Mainly, it seems
> easier to detect underflow cases during subtraction that way. And the factor
> of 2 in range doesn't change a whole lot.
I just realized it the other day :).
>>>> If you have time to look at the pg_test_timing part, it'd be
>>>> appreciated. That's a it larger, and nobody looked at it yet. So I'm a bit
>>>> hesitant to push it.
>>> I haven't yet pushed the pg_test_timing (nor it's small prerequisite)
>>> patch.
>>>
>>> I've attached those two patches. Feel free to include them in your series if
>>> you want, then the CF entry (and thus cfbot) makes sense again...
>> I'll include them in my new patch set and also have a careful look at them.

I reviewed the prerequisite patch which introduces 
INSTR_TIME_SET_SECONDS(), as well as the pg_test_timing patch. Here my 
comments:

- The prerequisite patch looks good me.

- By default, the test query in the pg_test_timing doc runs serially. 
What about adding SET max_parallel_workers_per_gather = 0 to make sure 
it really always does (e.g. on a system with different settings for 
parallel_tuple_cost / parallel_setup_cost)? Otherwise, the numbers will 
be much more flaky.

- Why have you added a case distinction for diff == 0? Have you 
encountered this case? If so, how? Maybe add a comment.

- To further reduce overhead we could call INSTR_TIME_SET_CURRENT() 
multiple times. But then again: why do we actually care about the 
per-loop time? Why not instead sum up diff and divide by the number of 
iterations to exclude all the overhead in the first place?

- In the computation of the per-loop time in nanoseconds you can now use 
INSTR_TIME_GET_NANOSEC() instead of INSTR_TIME_GET_DOUBLE() * NS_PER_S.

The rest looks good to me. The rebased patches are part of the patch set 
I sent out yesterday in reply to another mail in this thread.

-- 
David Geier
(ServiceNow)






view thread (172+ messages)  latest in thread

Message-ID: <3ac157f7-085d-e071-45fc-b87cd306360c@gmail.com>
Permalink:  ../3ac157f7-085d-e071-45fc-b87cd306360c@gmail.com/
Also on:    postgresql.org/message-id/3ac157f7-085d-e071-45fc-b87cd306360c@gmail.com

 · 

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: pgsql-hackers@postgresql.org
  Cc: geidav.pg@gmail.com, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, vignesh21@gmail.com, lukas@fittl.com, michael@paquier.xyz, ibrar.ahmad@gmail.com, m.sakrejda@gmail.com
  Subject: Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?
  In-Reply-To: <3ac157f7-085d-e071-45fc-b87cd306360c@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox