Re: unnecessary executor overheads around seqscans

Hi,

On 2026-01-24 15:21:22 +0900, Amit Langote wrote:
> On Sat, Jan 24, 2026 at 5:16 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > - The TupIsNull(slot) check in ExecScanExtended() is redundant with the return
> > value of table_scan_getnextslot(), but the compiler doesn't grok that.
> >
> > We can use a pg_assume() in table_scan_getnextslot() to make the compiler
> > understand.
>
> Something like this?
>
> result = sscan->rs_rd->rd_tableam->scan_getnextslot(sscan, direction, slot);
> pg_assume(result == !TupIsNull(slot));
> return result;

Yep, that seems to clue at least gcc into understanding the situation. I only
tried two pg_assume(!found || !TupIsNull(slot)), but yours should probably
also work.

> I assume this relies on table_scan_getnextslot() being inlined into
> ExecScanExtended()?

Yes. But I think that that's a good bet, given that it's an inline function
and only used once in nodeSeqscan.c.

> > - heap_getnextslot() calls ExecStoreBufferHeapTuple() and then returns
> > true. That prevents the sibiling call optimization.
> >
> > We should change ExecStoreBufferHeapTuple() to return true. Nobody uses the
> > current return value. Alternatively we should consider just moving it to
> > somewhere heapam.c/heapam_handler.c can see the implementations, they're the
> > only ones that should use it anyway.
>
> Makes sense. Changing ExecStoreBufferHeapTuple() to return true seems
> like the simpler option, unless I misunderstood.

Yea. There's probably more to be gained by the other approach, but it's also
somewhat painful due to some functionality being private to execTuples.c right
now.

Greetings,

Andres Freund