Repeated array type info caching code

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

Browse pgsql-hackers by date

  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])