Re: Re: patch: fix performance problems with repated decomprimation of varlena values in plpgsql

On Fri, Jan 28, 2011 at 2:19 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2011/1/28 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Tue, Jan 25, 2011 at 11:29 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>> variant a) my first patch - detoast on first usage with avoiding to
>>> useless detoast checking
>>> variant b) my first patch - detoast on first usage without avoiding to
>>> useless detoast checking
>>>
>>> time for 1 - about 300 ms, a is bout 1.5% faster than b
>>> time for 2 - about 30000 ms, a is about 3% faster than b
>>
>> This makes your approach sound pretty good, but it sounds like we
>> might need to find a better way to structure the code.
>>
>
> do you have a any idea?

At first blush, the should_be_detoasted flag looks completely
unnecessary to me. I don't see why we have to set a flag in one part
of the code to tell some other part of the code whether or not it
should detoast. Why is that necessary or a good idea? Why can't we
just make the decision at the point where we're detoasting?

Also, I believe I'm agreeing with Tom when I say that
exec_eval_datum() doesn't look like the right place to do this. Right
now that function is a very simple switch, and I've found that it's
usually a bad idea to stick complex code into the middle of such
things. It looks like function only has three callers, so maybe we
should look at each caller individually and see whether it needs this
logic or not. For example, I'm guessing make_tuple_from_row()
doesn't, because it's only calling exec_eval_datum() so that it can
pass the results to heap_form_tuple(), which already contains some
logic to detoast in certain cases. It's not clear why we'd want
different logic here than we do anywhere else. The other callers are
exec_assign_value(), where this looks like it could be important to
avoid repeatedly detoasting the array, and plpgsql_param_fetch(),
which I'm not sure about, but perhaps you have an idea or can
benchmark it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company