Horton
Menu

Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

Haibo Yan <tristan.yim@gmail.com>

View thread
From: Haibo Yan <tristan.yim@gmail.com>
To: Lukas Fittl <lukas@fittl.com>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers <pgsql-hackers@postgresql.org>, David Geier <geidav.pg@gmail.com>, andrew@dunslane.net, Maciek Sakrejda <m.sakrejda@gmail.com>
Date: 2026-04-12T06:04:42Z
Lists: pgsql-hackers 
On Fri, Apr 10, 2026 at 12:29 AM Lukas Fittl <lukas@fittl.com> wrote:

> On Fri, Apr 10, 2026 at 12:12 AM Lukas Fittl <lukas@fittl.com> wrote:
> >
> > On Thu, Apr 9, 2026 at 9:02 AM Andres Freund <andres@anarazel.de> wrote:
> > >
> > > On 2026-04-08 21:36:48 -0700, Lukas Fittl wrote:
> > > >
> > > > > What do you think about making pg_test_timing warn and return 1 if
> there is a
> > > > > tsc clocksource but the calibrated frequency differs by more than,
> idk, 10%?
> > > > > I'm worried that there might be other problems like this lurking
> and we
> > > > > wouldn't know about them unless the issue is of a similar
> magnitude.
> > > >
> > > > Yeah, that seems like a good idea. If I understand correctly you're
> > > > thinking we could tell the user to switch to
> > > > timing_clock_source=system in that case? (i.e. this is only a
> > > > pg_test_timing notice, not something "smarter" in the backend itself)
> > >
> > > I'd even just say "investigate your system an/or report a bug to
> postgres" :)
> > >
> >
> > Sure, seems reasonable. I went ahead and added that in the attached
> > v27 (squashed with your other change).
> >
> > Example how that looks like (tested without the fix in place):
> >
> > ---
> >
> > TSC frequency source: x86, hypervisor, cpuid 0x15
> > TSC frequency in use: 7 kHz
> > TSC frequency from calibration: 2500260 kHz
> > WARNING: Calibrated TSC frequency differs by 35717900.0% from the TSC
> > frequency in use
> > HINT: Consider setting timing_clock_source to 'system'. Report bugs to
> > <pgsql-bugs@lists.postgresql.org>.
> >
> > TSC clock source will be used by default, unless timing_clock_source
> > is set to 'system'.
> >
> > ---
> >
> > I also added the extra newline before the "will be used by default"
> > message, because I felt its too much information bunched together
> > otherwise.
>
> I just realized that you initially suggested to do a "return 1", but I
> did not include that in the version I sent. I think we could easily do
> an "exit(1)" after that warning is printed - seems sensible to get
> CI/buildfarm to fail clearly.
>
> Thanks,
> Lukas
>
> --
> Lukas Fittl
>
>
>
Hi Lukas,
Thanks for the patch. I may be missing something, but I wonder if the new
debug path can still end up calling pg_tsc_calibrate_frequency() after
tsc_detect_frequency() already bailed out with no rdtscp or not invariant.
If so, it seems the diagnostics path is no longer following the same gate
as the normal TSC-usability path, and could still execute pg_rdtscp() while
only trying to print debug info.
Maybe I am overlooking a guard somewhere, but I think it would be safer
either to use the same prerequisites here, or keep this helper purely
passive.
Best,
Haibo

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: Show additional TSC clock source debug info

  2. instrumentation: Avoid CPUID 0x15/0x16 for Hypervisor TSC frequency

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

  4. Allow retrieving x86 TSC frequency/flags from CPUID

  5. instrumentation: Standardize ticks to nanosecond conversion method

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

  7. Check for __cpuidex and __get_cpuid_count separately

  8. pg_test_timing: Reduce per-loop overhead

  9. Refactor handling of x86 CPUID instructions

  10. instrumentation: Drop INSTR_TIME_SET_CURRENT_LAZY macro

  11. Rename pg_crc32c_sse42_choose.c for general purpose

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

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

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