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
