I wrote:
>> Hmm, I don't like the trend here. For the repeat-1000x query, I get
>> these reported execution times:
>> 8.4 360 ms
>> 9.0 365 ms
>> 9.1 440 ms
>> 9.2 510 ms
>> 9.3 550 ms
>> 9.4 570 ms
>> head 570 ms
> I made a quick-hack patch to suppress redundant GetDefaultOpclass calls
> in typcache.c, and found that that brought HEAD's runtime down to 460ms.
I found some additional low-hanging fruit by comparing gprof call counts
in 8.4 and HEAD:
* OverrideSearchPathMatchesCurrent(), which is not there at all in 8.4
or 9.2, accounts for a depressingly large amount of palloc/pfree traffic.
The implementation was quick-n-dirty to begin with, but workloads
like this one call it often enough to make it a pain point.
* plpgsql's setup_param_list() contributes another large fraction of
added palloc/pfree traffic; this is evidently caused by the temporary
bitmapset needed for its bms_first_member() loop, which was not there
in 8.4 but is there in 9.2.
I've been able to bring HEAD's runtime down to about 415 ms with the
collection of more-or-less quick hacks attached. None of them are
ready to commit but I thought I'd post them for the record.
After review of all this, I think the aspect of your example that is
causing performance issues is that there are a lot of non-inline-able
SQL-language function calls. That's not a case that we've put much
thought into lately. I doubt we are going to get all the way back to
where 8.4 was in the short term, because I can see that there is a
significant amount of new computation associated with collation
management during parsing (catcache lookups driven by get_typcollation,
assign_collations_walker, etc). The long-term answer to that is to
improve the SQL-language function support so that it can cache the results
of parsing the function body; we have surely got enough plancache support
for that now, but no one's attempted to apply it in functions.c.
regards, tom lane
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 8c6c7fc..bf80563 100644
*** a/src/backend/utils/cache/typcache.c
--- b/src/backend/utils/cache/typcache.c
*************** static HTAB *TypeCacheHash = NULL;
*** 77,82 ****
--- 77,84 ----
#define TCFLAGS_CHECKED_FIELD_PROPERTIES 0x0010
#define TCFLAGS_HAVE_FIELD_EQUALITY 0x0020
#define TCFLAGS_HAVE_FIELD_COMPARE 0x0040
+ #define TCFLAGS_CHECKED_BTREE_OPCLASS 0x0080
+ #define TCFLAGS_CHECKED_HASH_OPCLASS 0x0100
/* Private information to support comparisons of enum values */
typedef struct
*************** lookup_type_cache(Oid type_id, int flags
*** 227,249 ****
{
Oid opclass;
! opclass = GetDefaultOpClass(type_id, BTREE_AM_OID);
! if (OidIsValid(opclass))
{
! typentry->btree_opf = get_opclass_family(opclass);
! typentry->btree_opintype = get_opclass_input_type(opclass);
}
/* If no btree opclass, we force lookup of the hash opclass */
if (typentry->btree_opf == InvalidOid)
{
if (typentry->hash_opf == InvalidOid)
{
! opclass = GetDefaultOpClass(type_id, HASH_AM_OID);
! if (OidIsValid(opclass))
{
! typentry->hash_opf = get_opclass_family(opclass);
! typentry->hash_opintype = get_opclass_input_type(opclass);
}
}
}
else
--- 229,259 ----
{
Oid opclass;
! if (!(typentry->flags & TCFLAGS_CHECKED_BTREE_OPCLASS))
{
! opclass = GetDefaultOpClass(type_id, BTREE_AM_OID);
! if (OidIsValid(opclass))
! {
! typentry->btree_opf = get_opclass_family(opclass);
! typentry->btree_opintype = get_opclass_input_type(opclass);
! }
! typentry->flags |= TCFLAGS_CHECKED_BTREE_OPCLASS;
}
/* If no btree opclass, we force lookup of the hash opclass */
if (typentry->btree_opf == InvalidOid)
{
if (typentry->hash_opf == InvalidOid)
{
! if (!(typentry->flags & TCFLAGS_CHECKED_HASH_OPCLASS))
{
! opclass = GetDefaultOpClass(type_id, HASH_AM_OID);
! if (OidIsValid(opclass))
! {
! typentry->hash_opf = get_opclass_family(opclass);
! typentry->hash_opintype = get_opclass_input_type(opclass);
! }
}
+ typentry->flags |= TCFLAGS_CHECKED_HASH_OPCLASS;
}
}
else
*************** lookup_type_cache(Oid type_id, int flags
*** 268,278 ****
{
Oid opclass;
! opclass = GetDefaultOpClass(type_id, HASH_AM_OID);
! if (OidIsValid(opclass))
{
! typentry->hash_opf = get_opclass_family(opclass);
! typentry->hash_opintype = get_opclass_input_type(opclass);
}
}
--- 278,292 ----
{
Oid opclass;
! if (!(typentry->flags & TCFLAGS_CHECKED_HASH_OPCLASS))
{
! opclass = GetDefaultOpClass(type_id, HASH_AM_OID);
! if (OidIsValid(opclass))
! {
! typentry->hash_opf = get_opclass_family(opclass);
! typentry->hash_opintype = get_opclass_input_type(opclass);
! }
! typentry->flags |= TCFLAGS_CHECKED_HASH_OPCLASS;
}
}
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 911f015..ca93307 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
*************** CopyOverrideSearchPath(OverrideSearchPat
*** 3145,3164 ****
bool
OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
{
! /* Easiest way to do this is GetOverrideSearchPath() and compare */
! bool result;
! OverrideSearchPath *cur;
! cur = GetOverrideSearchPath(CurrentMemoryContext);
! if (path->addCatalog == cur->addCatalog &&
! path->addTemp == cur->addTemp &&
! equal(path->schemas, cur->schemas))
! result = true;
! else
! result = false;
! list_free(cur->schemas);
! pfree(cur);
! return result;
}
/*
--- 3145,3178 ----
bool
OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
{
! ListCell *lc, *lcc;
! recomputeNamespacePath();
! lc = list_head(activeSearchPath);
! if (path->addTemp)
! {
! if (lc && lfirst_oid(lc) == myTempNamespace)
! lc = lnext(lc);
! else
! return false;
! }
! if (path->addCatalog)
! {
! if (lc && lfirst_oid(lc) == PG_CATALOG_NAMESPACE)
! lc = lnext(lc);
! else
! return false;
! }
! foreach(lcc, path->schemas)
! {
! if (lc && lfirst_oid(lc) == lfirst_oid(lcc))
! lc = lnext(lc);
! else
! return false;
! }
! if (lc)
! return false;
! return true;
}
/*
diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h
index f770608..a78ff48 100644
*** a/src/include/nodes/bitmapset.h
--- b/src/include/nodes/bitmapset.h
*************** extern Bitmapset *bms_join(Bitmapset *a,
*** 89,94 ****
--- 89,95 ----
/* support for iterating through the integer elements of a set: */
extern int bms_first_member(Bitmapset *a);
+ extern int bms_next_member(const Bitmapset *a, int prevbit);
/* support for hashtables using Bitmapsets as keys: */
extern uint32 bms_hash_value(const Bitmapset *a);
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index c927b78..fbaf377 100644
*** a/src/backend/nodes/bitmapset.c
--- b/src/backend/nodes/bitmapset.c
*************** bms_first_member(Bitmapset *a)
*** 841,846 ****
--- 841,899 ----
}
/*
+ * bms_next_member - find next member of a set
+ *
+ * Returns smallest member greater than "prevbit", or -1 if there is none.
+ *
+ * This is intended as support for iterating through the members of a set.
+ * The typical pattern is
+ *
+ * x = -1;
+ * while ((x = bms_next_member(inputset, x)) >= 0)
+ * process member x;
+ */
+ int
+ bms_next_member(const Bitmapset *a, int prevbit)
+ {
+ int nwords;
+ int wordnum;
+ bitmapword mask;
+
+ if (a == NULL)
+ return -1;
+ nwords = a->nwords;
+ prevbit++;
+ mask = (~(bitmapword) 0) << BITNUM(prevbit);
+ for (wordnum = WORDNUM(prevbit); wordnum < nwords; wordnum++)
+ {
+ bitmapword w = a->words[wordnum];
+
+ /* ignore bits before prevbit */
+ w &= mask;
+
+ if (w != 0)
+ {
+ int result;
+
+ w = RIGHTMOST_ONE(w);
+
+ result = wordnum * BITS_PER_BITMAPWORD;
+ while ((w & 255) == 0)
+ {
+ w >>= 8;
+ result += 8;
+ }
+ result += rightmost_one_pos[w & 255];
+ return result;
+ }
+
+ /* in subsequent words, consider all bits */
+ mask = (~(bitmapword) 0);
+ }
+ return -1;
+ }
+
+ /*
* bms_hash_value - compute a hash key for a Bitmapset
*
* Note: we must ensure that any two bitmapsets that are bms_equal() will
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 11cb47b..d6d414f 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** setup_param_list(PLpgSQL_execstate *esta
*** 5263,5269 ****
*/
if (!bms_is_empty(expr->paramnos))
{
- Bitmapset *tmpset;
int dno;
paramLI = (ParamListInfo)
--- 5263,5268 ----
*************** setup_param_list(PLpgSQL_execstate *esta
*** 5276,5283 ****
paramLI->numParams = estate->ndatums;
/* Instantiate values for "safe" parameters of the expression */
! tmpset = bms_copy(expr->paramnos);
! while ((dno = bms_first_member(tmpset)) >= 0)
{
PLpgSQL_datum *datum = estate->datums[dno];
--- 5275,5282 ----
paramLI->numParams = estate->ndatums;
/* Instantiate values for "safe" parameters of the expression */
! dno = -1;
! while ((dno = bms_next_member(expr->paramnos, dno)) >= 0)
{
PLpgSQL_datum *datum = estate->datums[dno];
*************** setup_param_list(PLpgSQL_execstate *esta
*** 5292,5298 ****
prm->ptype = var->datatype->typoid;
}
}
- bms_free(tmpset);
/*
* Set up link to active expr where the hook functions can find it.
--- 5291,5296 ----
--
Sent via pgsql-performance mailing list (pgsql-performance@xxxxxxxxxxxxxx)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance
