Horton

Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: David Geier <geidav.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, vignesh C <vignesh21@gmail.com>, Lukas Fittl <lukas@fittl.com>, Michael Paquier <michael@paquier.xyz>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Maciek Sakrejda <m.sakrejda@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-21T04:12:00Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. pg_test_timing: Also test RDTSC[P] timing, report time source, TSC frequency

  2. Allow retrieving x86 TSC frequency/flags from CPUID

  3. instrumentation: Standardize ticks to nanosecond conversion method

  4. instrumentation: Use Time-Stamp Counter on x86-64 to lower overhead

  5. Zero initialize uses of instr_time about to trigger compiler warnings

  6. instr_time: Represent time as an int64 on all platforms

  7. Add 250c8ee07ed to git-blame-ignore-revs

Hi,

On 2023-01-19 11:47:49 +0100, David Geier wrote:
> > I also couldn't help and hacked a bit on the rdtsc pieces. I did figure out
> > how to do the cycles->nanosecond conversion with integer shift and multiply in
> > the common case, which does show a noticable speedup. But that's for another
> > day.
> I also have code for that here. I decided against integrating it because we
> don't convert frequently enough to make it matter. Or am I missing
> something?

We do currently do the conversion quite frequently.  Admittedly I was
partially motivated by trying to get the per-loop overhead in pg_test_timing
down ;)

But I think it's a real issue. Places where we do, but shouldn't, convert:

- ExecReScan() - quite painful, we can end up with a lot of those
- InstrStopNode() - adds a good bit of overhead to simple
- PendingWalStats.wal_write_time - this is particularly bad because it happens
  within very contended code
- calls to pgstat_count_buffer_read_time(), pgstat_count_buffer_write_time() -
  they can be very frequent
- pgbench.c, as we already discussed
- pg_stat_statements.c
- ...

These all will get a bit slower when moving to a "variable" frequency.


What was your approach for avoiding the costly operation?  I ended up with a
integer multiplication + shift approximation for the floating point
multiplication (which in turn uses the inverse of the division by the
frequency). To allow for sufficient precision while also avoiding overflows, I
had to make that branch conditional, with a slow path for large numbers of
nanoseconds.


> > I fought a bit with myself about whether to send those patches in this thread,
> > because it'll take over the CF entry. But decided that it's ok, given that
> > David's patches should be rebased over these anyway?
> That's alright.
> Though, I would hope we attempt to bring your patch set as well as the RDTSC
> patch set in.

I think it'd be great - but I'm not sure we're there yet, reliability and
code-complexity wise.

I think it might be worth makign the rdts aspect somewhat
measurable. E.g. allowing pg_test_timing to use both at the same time, and
have it compare elapsed time with both sources of counters.

Greetings,

Andres Freund