Re: unnecessary executor overheads around seqscans

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Amit Langote <amitlangote09(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: unnecessary executor overheads around seqscans
Date: 2026-01-27 10:16:15
Message-ID: 0dc565fa-bcc0-4600-9cef-263822b08132@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 27/01/2026 01:25, Andres Freund wrote:
> On 2026-01-26 17:23:16 +0200, Heikki Linnakangas wrote:
>> Perhaps we should turn the ExecScanExtended() function inside out. Instead
>> of passing SeqNext as a callback to ExecScanExtended(), we would have a
>> function like this (for illustration purposes only, doesn't compile):
>
> That would be one approach, would require structural changes in a fair number
> of places though :/.

ExecScanExtended is only used in execScan.c and nodeSeqScan.c, so not
that many places. Even replacing ExecScan() completely would be isolated
to src/backend/executor.

> A slightly simpler approach could be for ExecScanExtended to pass in these
> parameters as arguments to the callbacks. For things like estate, direction
> and scanslot, that makes plenty sense. It's a bit more problematic for the
> scan descriptor, due to the "lazy start" we have in a few places.

Yep, it's less flexible. We have to know beforehand which variables the
function might want to avoid re-fetching, and add them all as parameters.

> I very briefly prototyped that (relying on the fact that all callers cast to
> the callback type and that passing unused arguments just works even if the
> function definition doesn't expect them), and that seems to do the trick.

This can be a very hot codepath so if we can shave a few % off, I'm
willing to hold my nose. But if we can do it more elegantly, even better...

> What shows up more visibly afterwards is that we set ExecScanExtended() sets
> econtext->ecxt_scantuple in every iteration, despite that not changing in
> almost all cases (I think for FDWs it could, fdwhandler.sgml just says that
> the scanslot "should" be used).

Huh, that's just a single store instruction. This really is a hot path.
Or is it just that the first instruction after the function call causes
some kind of a stall or data dependency, i.e. if that was removed, it'd
just move to the next instruction instead?

I wonder about the access to (econtext)->ecxt_per_tuple_memory. That
never changes, but we're re-fetching that too on every iteration.

InstrCountFiltered1() also fetches node->instrument, with a NULL check,
on every iteration. Maybe keep a counter in a local variable and only
call InstrCountFiltered1() when exiting the loop.

I guess these don't show up in a profiler or you would've latched on
them already..

- Heikki

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2026-01-27 10:23:43 Re: Time to add FIDO2 support?
Previous Message Joel Jacobson 2026-01-27 09:35:36 Re: Time to add FIDO2 support?