Thread

  1. Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

    Lukas Fittl <lukas@fittl.com> — 2025-03-01T07:45:58Z

    On Sun, Jun 2, 2024 at 1:08 AM Andres Freund <andres@anarazel.de> wrote:
    
    > At some point this patch switched from rdtsc to rdtscp, which imo largely
    > negates the point of it. What lead to that?
    
    
    From what I can gather, it appears this was an oversight when David first
    reapplied the work on the instr_time changes that were committed.
    
    I've come back to this and rebased this, as well as:
    - Corrected the use of RDTSCP to RDTSC in pg_get_ticks_fast
    - Check 16H register if 15H register does not contain frequency information
    (per research, relevant for some CPUs)
    - Fixed incorrect reporting in pg_test_timing due to too small histogram
    (32 => 64 bits)
    - Fixed indentation per pgindent
    - Added support for VMs running under KVM/VMware Hypervisors
    
    On that last item, this does indeed make a difference on VMs, contrary to
    the code comment in earlier versions (and I've not seen any odd behaviors
    again, FWIW):
    
    On a c5.xlarge (Skylake-SP or Cascade Lake) on AWS, with the same test as
    done initially in this thread:
    
    SELECT COUNT(*) FROM lotsarows;
    Time: 974.423 ms
    
    EXPLAIN (ANALYZE, TIMING OFF) SELECT COUNT(*) FROM lotsarows;
    Time: 1336.196 ms (00:01.336)
    
    Without patch:
    EXPLAIN (ANALYZE) SELECT COUNT(*) FROM lotsarows;
    Time: 2165.069 ms (00:02.165)
    
    Per loop time including overhead: 22.15 ns
    
    With patch:
    EXPLAIN (ANALYZE, TIMING ON) SELECT COUNT(*) FROM lotsarows;
    Time: 1654.289 ms (00:01.654)
    
    Per loop time including overhead: 9.81 ns
    
    I'm registering this again in the current commitfest to help reviews.
    
    Open questions I have:
    - Could we rely on checking whether the TSC timesource is invariant (via
    CPUID), instead of relying on Linux choosing it as a clocksource?
    - For the Hypervisor CPUID checks I had to rely on __cpuidex which is only
    available on newer GCC versions (
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95973), how do we best check
    for its presence? (compiler version, or rather configure check?) -- note
    this is also the reason the patch fails the clang compiler warning check in
    CI, despite clang having support in recent versions (
    https://reviews.llvm.org/D121653)
    
    Thanks,
    Lukas
    
    -- 
    Lukas Fittl