Re: Pinning a buffer in TupleTableSlot is unnecessary

On Wed, Aug 31, 2016 at 6:03 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> 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..

I have moved this patch to next CF. Heikki, are you still planning to
work on it?
--
Michael