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

2011-09-26 Thread Tom Lane
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

2011-09-23 Thread Robert Haas
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-09-23 Thread Pavel Stehule
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

2011-09-22 Thread Pavel Stehule
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

2011-09-22 Thread Robert Haas
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

2011-09-22 Thread Pavel Stehule
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

2011-09-16 Thread Tom Lane
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


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

2011-06-20 Thread Simon Riggs
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

2011-06-20 Thread Pavel Stehule
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