Re: patch: plpgsql - remove unnecessary ccache search when a array variable is updated

Hello

this patch significantly reduce a ccache searching. On my test - bubble sort

postgres=# \sf buble
CREATE OR REPLACE FUNCTION public.buble(integer[])
RETURNS integer[]
LANGUAGE plpgsql
AS $function$
declare
unsorted bool := true;
aux int;
begin
while unsorted
loop
unsorted := false;
for i in array_lower($1,1) .. array_upper($1,1) - 1
loop
if $1[i] > $1[i+1] then
aux := $1[i];
$1[i] := $1[i+1]; $1[i+1] := aux;
unsorted := true;
end if;
end loop;
end loop;
return $1;
end;
$function$ immutable

it decrease evaluation time about 15%.

Regards

Pavel Stehule

p.s. I know so bubble sort is not effective for large arrays. This
algorithm was used because a array is intensive modified.

On Mon, Jun 20, 2011 at 10:49 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> this patch significantly reduce a ccache searching. On my test - bubble sort

It sounds good, but also somewhat worrying.

The first cache is slow, so we add another cache to avoid searching
the first cache.

What is making the first cache so slow?

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Hello

2011/6/20 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On Mon, Jun 20, 2011 at 10:49 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>> this patch significantly reduce a ccache searching. On my test - bubble sort
>
> It sounds good, but also somewhat worrying.
>
> The first cache is slow, so we add another cache to avoid searching
> the first cache.
>
> What is making the first cache so slow?

a using of general cache should be slower than direct access to
memory. The slow down is based on catalog operations - hash
calculations, hash searching and cache validations. I don't know if it
is possible to optimize general cache.

you can compare profile of original pg

3008 13.0493 SearchCatCache
1306 5.6657 ExecEvalParamExtern
1143 4.9586 GetSnapshotData
1122 4.8675 AllocSetAlloc
1058 4.5898 MemoryContextAllocZero
1002 4.3469 ExecMakeFunctionResultNoSets
986 4.2775 ExecEvalArrayRef
851 3.6918 LWLockAcquire
783 3.3968 LWLockRelease
664 2.8806 RevalidateCachedPlan
646 2.8025 AllocSetFree
568 2.4641 array_ref
551 2.3904 CopySnapshot
519 2.2515 AllocSetReset
510 2.2125 array_set
492 2.1344 PopActiveSnapshot
381 1.6529 ArrayGetOffset
369 1.6008 AcquireExecutorLocks
348 1.5097 pfree
347 1.5054 MemoryContextAlloc
313 1.3579 bms_is_member
285 1.2364 CatalogCacheComputeHashValue
267 1.1583 PushActiveSnapshot
266 1.1540 hash_uint32
253 1.0976 pgstat_init_function_usage
233 1.0108 array_seek.clone.0

and patched postgresql's profile

3151 7.2135 AllocSetAlloc
2887 6.6091 ExecEvalParamExtern
2844 6.5107 list_member_ptr
2353 5.3867 AllocSetFree
2318 5.3065 GetSnapshotData
2201 5.0387 ExecMakeFunctionResultNoSets
2153 4.9288 LWLockAcquire
2055 4.7045 ExecEvalArrayRef
1879 4.3015 LWLockRelease
1675 3.8345 MemoryContextAllocZero
1463 3.3492 AcquireExecutorLocks
1375 3.1477 pfree
1356 3.1043 RevalidateCachedPlan
1261 2.8868 AllocSetCheck
1257 2.8776 PopActiveSnapshot
1115 2.5525 array_set
1102 2.5228 AllocSetReset
966 2.2114 CopySnapshot
938 2.1473 MemoryContextAlloc
875 2.0031 array_ref
772 1.7673 ResourceOwnerForgetPlanCacheRef
632 1.4468 array_seek.clone.0
554 1.2683 PushActiveSnapshot
499 1.1423 check_list_invariants
475 1.0874 ExecEvalConst
473 1.0828 bms_is_member
444 1.0164 ArrayGetNItems

so the most slow operation is SearchCatCache - but I am not a man who
can optimize this routine :)

Regards

Pavel Stehule

>
> --
>  Simon Riggs                   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> this patch significantly reduce a ccache searching.

I looked at this patch a little bit. It's got a very serious problem:
it supposes that the parent of an ARRAYELEM datum must be a VAR datum,
which is not so. As an example, it gets an Assert failure on this:

create table rtype (id int, ar text[]);

create or replace function foo() returns text[] language plpgsql as $$
declare
r record;
begin
r := row(12, '{foo,bar,baz}')::rtype;
r.ar[2] := 'replace';
return r.ar;
end$$;

select foo();

There is not any good place to keep the array element lookup data for
the non-VAR cases that is comparable to what you did for VAR. I wasn't
exactly thrilled about adding another field to PLpgSQL_var anyway,
because it would go unused in the large majority of cases.

A possible solution is to use the ARRAYELEM datum itself to hold the
cached lookup data. I'm not sure if it's worth having a level of
indirection as you do here; you might as well just drop the fields right
into PLpgSQL_arrayelem, because they'd be used in the vast majority of
cases.

Also, in order to deal with subscripting record fields, you'd better be
prepared for the possibility that the target array type changes from
time to time. I'd envision this working similarly to what various
array-manipulating functions do: you remember the last input OID you
looked up, and whenever that changes, repeat the lookup steps.

regards, tom lane

note: some basic test shows about 15% speedup

Regards

Pavel Stehule

On Thu, Sep 22, 2011 at 5:10 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> note: some basic test shows about 15% speedup

Eh that's good, but I think you need to fix the fact that it crashes...

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

Hello

2011/9/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Thu, Sep 22, 2011 at 5:10 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> note: some basic test shows about 15% speedup
>
> Eh that's good, but I think you need to fix the fact that it crashes...
>

I fixed crash that described Tom. Do you know about other?

Regards

Pavel

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

On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> I fixed crash that described Tom. Do you know about other?

No, I just don't see a new version of the patch.

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

2011/9/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> I fixed crash that described Tom. Do you know about other?
>
> No, I just don't see a new version of the patch.
>

sorry - my mistake - I sent it only to Tom

Regards

Pavel

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

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> 2011/9/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>> I fixed crash that described Tom. Do you know about other?

>> No, I just don't see a new version of the patch.

> sorry - my mistake - I sent it only to Tom

Applied with corrections --- mostly, that you didn't think through the
domain-over-array case.

regards, tom lane