Re: unnecessary executor overheads around seqscans

Hi,

Following up on the earlier discussion:

https://www.postgresql.org/message-id/xzflwwjtwxin3dxziyblrnygy3gfygo5dsuw6ltcoha73ecmnf%40nh6nonzta7kw

Andres pointed out that the TupIsNull(slot) check in
ExecScanExtended() is redundant with the result of
table_scan_getnextslot(), but that the compiler cannot infer this
relationship.

Amit proposed adding a pg_assume(), and David confirmed through
objdump that GCC could then remove the redundant check. I noticed
that this change is still not present on current master, so I
prepared the attached patch.

The patch adds the following one-way invariant:

found = sscan->rs_rd->rd_tableam->scan_getnextslot(sscan,
direction,
slot);
pg_assume(!found || !TupIsNull(slot));

return found;

I used the implication form mentioned by Andres instead of:

found == !TupIsNull(slot)

The executor only requires that a successful call leave the slot
non-empty. It does not need to require the slot to be empty whenever
the table AM returns false.

The only in-tree scan_getnextslot implementation is
heap_getnextslot(). It clears the slot before returning false and
calls ExecStoreBufferHeapTuple() before returning true. The latter
clears TTS_FLAG_EMPTY. I also updated the TableAmRoutine callback
comment to document the successful-call contract.

With GCC 12.2.1 on AArch64 at -O2, objdump shows that the
TTS_FLAG_EMPTY load and branch following the table AM callback are
removed. I observed the same optimization with GCC 16. Apple Clang
21 did not eliminate the check in this code shape.

I also ran an in-memory microbenchmark using a single-client count(*)
SeqScan over a 20 million-row, 692 MB heap table, with parallel query
and JIT disabled. Across four alternating baseline/patched pairs of
100 scans each, the paired median improvement was about 0.53%, but
the result remained within measurement noise. This is consistent
with Amit's earlier observation, so I do not claim a measurable
runtime improvement; the demonstrated benefit is the simpler
generated code.

Both regular and assertion-enabled GCC builds passed:

- core regression tests: 243/243
- test_extensible: 1/1

Does this look worthwhile as a small Table AM/executor
code-generation cleanup?

Regards,
Xiaoyu