Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
Pavel Stehule pavel.steh...@gmail.com writes: 2011/9/23 Robert Haas robertmh...@gmail.com: On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
2011/9/23 Robert Haas robertmh...@gmail.com:
On Fri, Sep 23, 2011 at 12:26 AM, Pavel Stehule pavel.steh...@gmail.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
*** ./src/pl/plpgsql/src/gram.y.orig 2011-09-19 10:25:28.0 +0200
--- ./src/pl/plpgsql/src/gram.y 2011-09-22 22:02:11.0 +0200
***
*** 998,1003
--- 998,1004
new-dtype = PLPGSQL_DTYPE_ARRAYELEM;
new-subscript = $3;
new-arrayparentno = $1;
+ new-arraytypoid = InvalidOid;
plpgsql_adddatum((PLpgSQL_datum *) new);
*** ./src/pl/plpgsql/src/pl_exec.c.orig 2011-09-19 10:25:28.0 +0200
--- ./src/pl/plpgsql/src/pl_exec.c 2011-09-22 22:14:46.0 +0200
***
*** 3991,4008
PLpgSQL_expr *subscripts[MAXDIM];
int subscriptvals[MAXDIM];
bool oldarrayisnull;
! Oid arraytypeid,
! arrayelemtypeid;
int32 arraytypmod;
- int16 arraytyplen,
- elemtyplen;
- bool elemtypbyval;
- char elemtypalign;
Datum oldarraydatum,
coerced_value;
ArrayType *oldarrayval;
ArrayType *newarrayval;
SPITupleTable *save_eval_tuptable;
/*
* We need to do subscript evaluation, which might require
--- 3991,4004
PLpgSQL_expr *subscripts[MAXDIM];
int subscriptvals[MAXDIM];
bool oldarrayisnull;
! Oid arraytypoid;
int32 arraytypmod;
Datum oldarraydatum,
coerced_value;
ArrayType *oldarrayval;
ArrayType *newarrayval;
SPITupleTable *save_eval_tuptable;
+ PLpgSQL_arrayelem *base_arrayelem;
/*
* We need to do subscript evaluation, which might require
***
*** 4027,4032
--- 4023,4030
{
PLpgSQL_arrayelem *arrayelem = (PLpgSQL_arrayelem *) target;
+ base_arrayelem = arrayelem;
+
if (nsubscripts = MAXDIM)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
***
*** 4038,4061
/* Fetch current value of array datum */
exec_eval_datum(estate, target,
! arraytypeid, arraytypmod,
oldarraydatum, oldarrayisnull);
! /* If target is domain over array, reduce to base type */
! arraytypeid = getBaseTypeAndTypmod(arraytypeid, arraytypmod);
! /* ... and identify the element type */
! arrayelemtypeid = get_element_type(arraytypeid);
! if (!OidIsValid(arrayelemtypeid))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg(subscripted object is not an array)));
! get_typlenbyvalalign(arrayelemtypeid,
! elemtyplen,
! elemtypbyval,
! elemtypalign);
! arraytyplen = get_typlen(arraytypeid);
/*
* Evaluate the subscripts, switch into left-to-right order.
--- 4036,4067
/* Fetch current value of array datum */
exec_eval_datum(estate, target,
! arraytypoid, arraytypmod,
oldarraydatum, oldarrayisnull);
! /* If base_arrayelem has initialized metadata, then use it */
! if (base_arrayelem-arraytypoid != arraytypoid ||
! base_arrayelem-arraytypmod != arraytypmod)
! {
! /* If target is domain over array, reduce to base type */
! arraytypoid = getBaseTypeAndTypmod(arraytypoid, arraytypmod);
! base_arrayelem-arraytypoid = arraytypoid;
! base_arrayelem-arraytypmod = arraytypmod;
! /* ... and identify the element type */
! base_arrayelem-arrayelemtypoid = get_element_type(arraytypoid);
! if (!OidIsValid(base_arrayelem-arrayelemtypoid))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg(subscripted object is not an array)));
!
! get_typlenbyvalalign(base_arrayelem-arrayelemtypoid,
! (base_arrayelem-elemtyplen),
! (base_arrayelem-elemtypbyval),
! (base_arrayelem-elemtypalign));
! base_arrayelem-arraytyplen = get_typlen(arraytypoid);
! }
/*
* Evaluate the subscripts, switch into left-to-right order.
***
*** 4093,4099
/* Coerce source value to match array element type. */
coerced_value = exec_simple_cast_value(value,
valtype,
! arrayelemtypeid,
arraytypmod,
*isNull);
--- 4099,4105
/* Coerce source value to match array element type. */
coerced_value = exec_simple_cast_value(value,
valtype,
! base_arrayelem-arrayelemtypoid,
arraytypmod,
*isNull);
***
*** 4107,4118
* array, either, so that's a no-op too. This is all ugly but
* corresponds to the current
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
note: some basic test shows about 15% speedup Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
On Thu, Sep 22, 2011 at 5:10 PM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
Hello 2011/9/23 Robert Haas robertmh...@gmail.com: On Thu, Sep 22, 2011 at 5:10 PM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
Pavel Stehule pavel.steh...@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 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.
*** ./pl_exec.c.orig 2011-06-20 08:55:37.0 +0200
--- ./pl_exec.c 2011-06-20 11:38:55.988650907 +0200
***
*** 153,158
--- 153,170
static void exec_assign_value(PLpgSQL_execstate *estate,
PLpgSQL_datum *target,
Datum value, Oid valtype, bool *isNull);
+ static void
+ exec_eval_array_datum(PLpgSQL_execstate *estate,
+ PLpgSQL_datum *datum,
+ Oid *arraytypoid,
+ int32 *arraytypmod,
+ int16 *arraytyplen,
+ Oid *elemtypeid,
+ int16 *elemtyplen,
+ bool *elemtybyval,
+ char *elemtypalign,
+ Datum *value,
+ bool *isnull);
static void exec_eval_datum(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
Oid *typeid,
***
*** 3981,4001
arraytypeid, arraytypmod,
oldarraydatum, oldarrayisnull);
! /* If target is domain over array, reduce to base type */
! arraytypeid = getBaseTypeAndTypmod(arraytypeid, arraytypmod);
!
! /* ... and identify the element type */
! arrayelemtypeid = get_element_type(arraytypeid);
! if (!OidIsValid(arrayelemtypeid))
! ereport(ERROR,
! (errcode(ERRCODE_DATATYPE_MISMATCH),
! errmsg(subscripted object is not an array)));
!
! get_typlenbyvalalign(arrayelemtypeid,
! elemtyplen,
! elemtypbyval,
! elemtypalign);
! arraytyplen = get_typlen(arraytypeid);
/*
* Evaluate the subscripts, switch into left-to-right order.
--- 3993,4009
arraytypeid, arraytypmod,
oldarraydatum, oldarrayisnull);
! /* Fetch current value of array datum and metadata */
! exec_eval_array_datum(estate, target,
! arraytypeid,
! arraytypmod,
! arraytyplen,
! arrayelemtypeid,
! elemtyplen,
! elemtypbyval,
! elemtypalign,
! oldarraydatum,
! oldarrayisnull);
/*
* Evaluate the subscripts, switch into left-to-right order.
***
*** 4099,4104
--- 4107,4178
}
}
+
+ /*
+ * exec_eval_array_datum Get current value of PLpgSQL_datum
+ *
+ * The type oid, typmod, value in Datum format, and null flag are returned.
+ * The arraytyplen, elemtyplen, elemtypbyval, elemtypalign are returned.
+ *
+ * This routine is is used for array type variables - returns additional info
+ * about array elements - removes useless search inside ccache.
+ */
+ static void
+ exec_eval_array_datum(PLpgSQL_execstate *estate,
+ PLpgSQL_datum *datum,
+ Oid *arraytypoid,
+ int32 *arraytypmod,
+ int16 *arraytyplen,
+ Oid *elemtypeid,
+ int16 *elemtyplen,
+ bool *elemtybyval,
+ char *elemtypalign,
+ Datum *value,
+ bool *isnull)
+ {
+ PLpgSQL_var *var = (PLpgSQL_var *) datum;
+
+ Assert(datum-dtype == PLPGSQL_DTYPE_VAR);
+
+ if (var-array_desc == NULL)
+ {
+ PLpgSQL_array_desc *array_desc = (PLpgSQL_array_desc *) palloc(sizeof(PLpgSQL_array_desc));
+
+ array_desc-arraytypoid = var-datatype-typoid;
+ array_desc-arraytypmod = var-datatype-atttypmod;
+
+ /* If target is domain over array, reduce to base type */
+ array_desc-arraytypoid = getBaseTypeAndTypmod(array_desc-arraytypoid,
+ array_desc-arraytypmod);
+
+ array_desc-elemtypoid = get_element_type(array_desc-arraytypoid);
+ if (!OidIsValid(array_desc-elemtypoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg(subscripted object is not an array)));
+
+ get_typlenbyvalalign(array_desc-elemtypoid,
+ array_desc-elemtyplen,
+ array_desc-elemtypbyval,
+ array_desc-elemtypalign);
+ array_desc-arraytyplen = get_typlen(array_desc-arraytypoid);
+
+ var-array_desc = array_desc;
+ }
+
+ *arraytypoid = var-array_desc-arraytypoid;
+ *arraytypmod = var-array_desc-arraytypmod;
+ *arraytyplen = var-array_desc-arraytyplen;
+
+ *elemtypeid = var-array_desc-elemtypoid;
+ *elemtyplen = var-array_desc-elemtyplen;
+ *elemtybyval = var-array_desc-elemtypbyval;
+ *elemtypalign = var-array_desc-elemtypalign;
+
+ *value = var-value;
+ *isnull = var-isnull;
+ }
+
/*
* exec_eval_datumGet current value of a PLpgSQL_datum
*
***
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
On Mon, Jun 20, 2011 at 10:49 AM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: plpgsql - remove unnecessary ccache search when a array variable is updated
Hello 2011/6/20 Simon Riggs si...@2ndquadrant.com: On Mon, Jun 20, 2011 at 10:49 AM, Pavel Stehule pavel.steh...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
