From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Greg Stark <stark(at)mit(dot)edu>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Pinning a buffer in TupleTableSlot is unnecessary |
Date: | 2016-10-03 03:51:05 |
Message-ID: | CAB7nPqTMLXpaiA3M2q1iT7VTJDPPYKwM5d_gzThT5h3b7yFE9Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2016-10-03 03:51:53 | Re: Stopping logical replication protocol |
Previous Message | Craig Ringer | 2016-10-03 03:48:51 | Re: PATCH: Batch/pipelining support for libpq |