From: | Craig Ringer <craig(at)2ndquadrant(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Repeated array type info caching code |
Date: | 2013-07-26 05:33:07 |
Message-ID: | 51F20A13.7060308@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi all
While reading the array functions in varlena.c, arrayfuncs.c and
array_userfuncs.c, I noticed code to cache the array type info in
fn_extra repeated three times, and in adding an "array_idx" function
(generic version of intarray's 'idx') I'm expecting to need a fourth copy.
The ArrayMetaState type is already shared between all these usages. Is
there any reason the lookup its self isn't?
Would it be reasonable to define something along the lines of:
get_cached_arrmetastate(arrayv, fcinfo);
to populate a ArrayMetaState pointed to by fc_extra, allocating it if
necessary? So the resulting code would instead become at most:
ArrayMetaState * my_extra;
ArrayType * v = PG_GETARG_ARRAYTYPE_P(0);
int16 typlen;
bool typbyval;
char typalign;
/* ... */
my_extra = get_cached_arrmetastate(v, fcinfo);
typlen = my_extra->typlen;
typbyval = my_extra->typbyval;
typalign = my_extra->typalign;
instead of:
my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
if (my_extra == NULL)
{
fcinfo->flinfo->fn_extra =
MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
sizeof(ArrayMetaState)
);
my_extra = (ArrayMetaState *) fcinfo->flinfo->fn_extra;
my_extra->element_type = ~element_type;
}
if (my_extra->element_type != element_type)
{
/*
* Get info about element type, including its output
* conversion proc
*/
get_type_io_data(element_type, IOFunc_output,
&my_extra->typlen, &my_extra->typbyval,
&my_extra->typalign, &my_extra->typdelim,
&my_extra->typioparam, &my_extra->typiofunc);
fmgr_info_cxt(my_extra->typiofunc, &my_extra->proc,
fcinfo->flinfo->fn_mcxt);
my_extra->element_type = element_type;
}
typlen = my_extra->typlen;
typbyval = my_extra->typbyval;
typalign = my_extra->typalign;
?
If nobody yells "idiot" I'll follow up with a patch in a bit, as the
alternative of copying and pasting that code squicks me a bit.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | didier | 2013-07-26 05:45:02 | Re: Design proposal: fsync absorb linear slider |
Previous Message | Alvaro Herrera | 2013-07-26 05:04:51 | Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review]) |