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
