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: Michael Paquier <michael@paquier.xyz>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Maciek Sakrejda <m.sakrejda@gmail.com>, Lukas Fittl <lukas@fittl.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-21T04:29:10Z
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-18 14:02:48 +0100, David Geier wrote:
> On 1/16/23 18:37, Andres Freund wrote:
> > I'm doubtful this is worth the complexity it incurs. By the time we convert
> > out of the instr_time format, the times shouldn't be small enough that the
> > accuracy is affected much.
>
> I don't feel strong about it and you have a point that we most likely only
> convert ones we've accumulated a fair amount of cycles.

I think we can avoid the issue another way. The inaccuracy comes from the
cycles_to_sec ending up very small, right? Right now your patch has (and
probably my old version similarly had):

cycles_to_sec = 1.0 / (tsc_freq * 1000);

I think it's better if we have one multiplier to convert cycles to nanoseconds
- that'll be a double comparatively close to 1. We can use that to implement
INSTR_TIME_GET_NANOSECONDS(). The conversion to microseconds then is just a
division by 1000 (which most compilers convert into a multiplication/shift
combo), and the conversions to milliseconds and seconds will be similar.

Because we'll never "wrongly" go into the "huge number" or "very small number"
ranges, that should provide sufficient precision? We'll of course still end up
with a very small number when converting a few nanoseconds to seconds, but
that's ok because it's the precision being asked for, instead of loosing
precision in some intermediate representation.


> > Looking around, most of the existing uses of INSTR_TIME_GET_MICROSEC()
> > actually accumulate themselves, and should instead keep things in the
> > instr_time format and convert later. We'd win more accuracy / speed that way.
> > 
> > I don't think the introduction of pg_time_usec_t was a great idea, but oh
> > well.
> Fully agreed. Why not replacing pg_time_usec_t with instr_time in a separate
> patch?

pgbench used to use instr_time, but it was replaced by somebody thinking the
API is too cumbersome. Which I can't quite deny, even though I think the
specific change isn't great.

But yes, this should definitely be a separate patch.


Greetings,

Andres Freund