While profiling some queries and looking at executor overhead, I
realized that we're not making much use of TupleTableSlot's ability to
hold a buffer pin. In a SeqScan, the buffer is held pinned by the
underlying heap-scan anyway. Same with an IndexScan, and the SampleScan.
The only thing that relies on that feature is TidScan, but we could
easily teach TidScan to hold the buffer pin directly.
So, how about we remove the ability of a TupleTableSlot to hold a buffer
pin, per the attached patch? It shaves a few cycles from a
ExecStoreTuple() and ExecClearTuple(), which get called a lot. I
couldn't measure any actual difference from that, though, but it seems
like a good idea from a readability point of view, anyway.
How much do we need to worry about breaking extensions? I.e. to what
extent do we consider the TupleTableSlot API to be stable, for
extensions to use? The FDW API uses TupleTableSlots - this patch had to
fix the ExecStoreTuple calls in postgres_fdw.
We could refrain from changing the signature of ExecStoreTuple(), and
throw an error if you try to pass a valid buffer to it. But I also have
some bigger changes to TupleTableSlot in mind. I think we could gain
some speed by making slots "read-only". For example, it would be nice if
a SeqScan could just replace the tts_tuple pointer in the slot, and not
have to worry that the upper nodes might have materialized the slot, and
freeing the copied tuple. Because that happens very rarely in practice.
It would be better if those few places where we currently materialize an
existing slot, we would create a copy of the slot, and leave the
original slot unmodified. I'll start a different thread on that after
some more experimentation, but if we e.g. get rid of
ExecMaterializeSlot() in its current form, would that be OK?
- Heikki
