Re: [HACKERS] Faster setup_param_list() in plpgsql
I wrote:
> What this patch does is to remove setup_param_list() overhead for the
> common case of PLPGSQL_DTYPE_VAR variables (ie, any non-composite type).
> It does that by the expedient of keeping the ParamExternData image of such
> a variable valid at all times. That adds a few cycles to assignments to
> these variables, but removes more cycles from each use of them. Unless
> you believe that common plpgsql functions contain lots of dead stores,
> this is a guaranteed win overall.
> I'm seeing about 10% overall speedup (vs HEAD, with casserts off) for
> realistic simple plpgsql logic, such as this test case:
Here is a version that is rebased up to HEAD. Dunno if anyone wants
to re-review this, if not I'll go commit it.
regards, tom lane
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 0ff2086..05268e3 100644
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*** PLpgSQL_stmt_block *plpgsql_parse_result
*** 42,48
static int datums_alloc;
int plpgsql_nDatums;
PLpgSQL_datum **plpgsql_Datums;
! static int datums_last = 0;
char *plpgsql_error_funcname;
bool plpgsql_DumpExecTree = false;
--- 42,48
static int datums_alloc;
int plpgsql_nDatums;
PLpgSQL_datum **plpgsql_Datums;
! static int datums_last;
char *plpgsql_error_funcname;
bool plpgsql_DumpExecTree = false;
*** static Node *make_datum_param(PLpgSQL_ex
*** 104,109
--- 104,111
static PLpgSQL_row *build_row_from_class(Oid classOid);
static PLpgSQL_row *build_row_from_vars(PLpgSQL_variable **vars, int numvars);
static PLpgSQL_type *build_datatype(HeapTuple typeTup, int32 typmod, Oid collation);
+ static void plpgsql_start_datums(void);
+ static void plpgsql_finish_datums(PLpgSQL_function *function);
static void compute_function_hashkey(FunctionCallInfo fcinfo,
Form_pg_proc procStruct,
PLpgSQL_func_hashkey *hashkey,
*** do_compile(FunctionCallInfo fcinfo,
*** 371,383
plpgsql_ns_init();
plpgsql_ns_push(NameStr(procStruct->proname));
plpgsql_DumpExecTree = false;
!
! datums_alloc = 128;
! plpgsql_nDatums = 0;
! /* This is short-lived, so needn't allocate in function's cxt */
! plpgsql_Datums = MemoryContextAlloc(compile_tmp_cxt,
! sizeof(PLpgSQL_datum *) * datums_alloc);
! datums_last = 0;
switch (function->fn_is_trigger)
{
--- 373,379
plpgsql_ns_init();
plpgsql_ns_push(NameStr(procStruct->proname));
plpgsql_DumpExecTree = false;
! plpgsql_start_datums();
switch (function->fn_is_trigger)
{
*** do_compile(FunctionCallInfo fcinfo,
*** 758,767
function->fn_nargs = procStruct->pronargs;
for (i = 0; i < function->fn_nargs; i++)
function->fn_argvarnos[i] = in_arg_varnos[i];
! function->ndatums = plpgsql_nDatums;
! function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
! for (i = 0; i < plpgsql_nDatums; i++)
! function->datums[i] = plpgsql_Datums[i];
/* Debug dump for completed functions */
if (plpgsql_DumpExecTree)
--- 754,761
function->fn_nargs = procStruct->pronargs;
for (i = 0; i < function->fn_nargs; i++)
function->fn_argvarnos[i] = in_arg_varnos[i];
!
! plpgsql_finish_datums(function);
/* Debug dump for completed functions */
if (plpgsql_DumpExecTree)
*** plpgsql_compile_inline(char *proc_source
*** 804,810
PLpgSQL_variable *var;
int parse_rc;
MemoryContext func_cxt;
- int i;
/*
* Setup the scanner input and error info. We assume that this function
--- 798,803
*** plpgsql_compile_inline(char *proc_source
*** 860,870
plpgsql_ns_init();
plpgsql_ns_push(func_name);
plpgsql_DumpExecTree = false;
!
! datums_alloc = 128;
! plpgsql_nDatums = 0;
! plpgsql_Datums = palloc(sizeof(PLpgSQL_datum *) * datums_alloc);
! datums_last = 0;
/* Set up as though in a function returning VOID */
function->fn_rettype = VOIDOID;
--- 853,859
plpgsql_ns_init();
plpgsql_ns_push(func_name);
plpgsql_DumpExecTree = false;
! plpgsql_start_datums();
/* Set up as though in a function returning VOID */
function->fn_rettype = VOIDOID;
*** plpgsql_compile_inline(char *proc_source
*** 911,920
* Complete the function's info
*/
function->fn_nargs = 0;
! function->ndatums = plpgsql_nDatums;
! function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
! for (i = 0; i < plpgsql_nDatums; i++)
! function->datums[i] = plpgsql_Datums[i];
/*
* Pop the error context stack
--- 900,907
* Complete the function's info
*/
function->fn_nargs = 0;
!
! plpgsql_finish_datums(function);
/*
* Pop the error context stack
*** plpgsql_build_record(const char *refname
*** 1965,1970
--- 1952,1958
rec->tup = NULL;
rec->tupdesc = NULL;
rec->freetup = false;
+ rec->fre
Re: [HACKERS] Faster setup_param_list() in plpgsql
Review: What this patch does - it change a mechanism, how a values of variables are transmitted to SPI. In previous variant values are copied to ParamListInfo before every evaluation of any expression. New mechanism is smarter. It refresh only ROW, REC values when are marked as dirty (when these variables was used). ParamListInfo is dynamically updated when value is assigned to variable. This patch can significantly reduce a overhead related to preparing parameters - more it cleaning code. 1. There are no problem with patching, regress tests 2. The changes are clean and well documented 3. I can confirm 10% of speedup. This patch is ready for commit. Regards Pavel Stehule 2015-04-30 20:50 GMT+02:00 Pavel Stehule : > > > 2015-04-29 9:26 GMT+02:00 Pavel Stehule : > >> Hi all >> >> I am looking on this patch. I can confirm 10-15% speedup - and the idea >> behind this patch looks well. >> >> This patch >> http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us >> contains two parts >> >> a) relative large refactoring >> >> b) support for resettable fields in param_list instead total reset >> >> I believe it should be in two separate patches. Refactoring is trivial >> and there is no any possible objection. >> > > I was wrong, there is relative strong dependency between these two parts, > so it should be commited as one patch > > Regards > > Pavel > > >> >> Regards >> >> Pavel >> > >
Re: [HACKERS] Faster setup_param_list() in plpgsql
2015-04-29 9:26 GMT+02:00 Pavel Stehule : > Hi all > > I am looking on this patch. I can confirm 10-15% speedup - and the idea > behind this patch looks well. > > This patch > http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us > contains two parts > > a) relative large refactoring > > b) support for resettable fields in param_list instead total reset > > I believe it should be in two separate patches. Refactoring is trivial and > there is no any possible objection. > I was wrong, there is relative strong dependency between these two parts, so it should be commited as one patch Regards Pavel > > Regards > > Pavel >
Re: [HACKERS] Faster setup_param_list() in plpgsql
On 03/14/2015 06:04 PM, Tom Lane wrote: Given the rather hostile response I got to http://www.postgresql.org/message-id/4146.1425872...@sss.pgh.pa.us I was not planning to bring this topic up again until 9.6 development starts. However, as I said in that thread, this work is getting done now because of $dayjob deadlines, and I've realized that it would actually make a lot of sense to apply it before my expanded-arrays patch that's pending in the current commitfest. So I'm going to put on my flameproof long johns and post it anyway. I will add it to the 2015-06 commitfest, but I'd really rather deal with it now ... What this patch does is to remove setup_param_list() overhead for the common case of PLPGSQL_DTYPE_VAR variables (ie, any non-composite type). It does that by the expedient of keeping the ParamExternData image of such a variable valid at all times. That adds a few cycles to assignments to these variables, but removes more cycles from each use of them. Unless you believe that common plpgsql functions contain lots of dead stores, this is a guaranteed win overall. I'm seeing about 10% overall speedup (vs HEAD, with casserts off) for realistic simple plpgsql logic, such as this test case: create or replace function typicalspeed(n int) returns bigint as $$ declare res bigint := 0; begin for i in 1 .. n loop res := res + i; if i % 10 = 0 then res := res / 10; end if; end loop; return res; end $$ language plpgsql strict stable; For functions with lots of variables (or even just lots of expressions, since each one of those is a PLpgSQL_datum too), it's even more helpful. I have found no cases where it makes things worse, at least to within measurement error (run-to-run variability is a percent or two for me). The reason I would like to apply this now rather than wait for 9.6 is that by making parameter management more explicit it removes the need for the klugy changes in exec_eval_datum() that exist in http://www.postgresql.org/message-id/22945.1424982...@sss.pgh.pa.us Instead, we could leave exec_eval_datum() alone and substitute read-only pointers only when manufacturing the parameter image of an expanded-object variable. If we do it in the other order then we'll be making an API change for exec_eval_datum() in 9.5 (assuming said patch gets in) and then reverting it come 9.6. So there you have it. Now, where'd I put those long johns ... I'm inclined to say go for it. I can recall cases in the past where we have found some significant piece of work to be necessary after feature freeze in order to enable a piece of work submitted before feature freeze to proceed. This sounds like a similar case. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
