Re: Pinning a buffer in TupleTableSlot is unnecessary

On 2016-08-30 21:59:44 +0100, Greg Stark wrote:
> On Tue, Aug 30, 2016 at 11:12 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> > 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.
>
> Out of curiosity why go in this direction and not the other? Why not
> move those other scans to use the TupleTableSlot API to manage the
> pins. Offhand it sounds more readable not less to have the
> TupleTableSlot be a self contained data structure that guarantees the
> lifetime of the pins matches the slots rather than have a dependency
> on the code structure in some far-away scan.

At least for heap scans the pins are page level, and thus longer lived
than the data in a slot. It's important that a scan holds a pin, because
it needs to rely on the page not being hot pruned etc..