While working on the patch in [1], I noticed that ever since
00b41463c, it's now suboptimal to do the following:
switch (bms_membership(relids))
{
case BMS_EMPTY_SET:
/* handle empty set */
break;
case BMS_SINGLETON:
/* call bms_singleton_member() and handle singleton set */
break;
case BMS_MULTIPLE:
/* handle multi-member set */
break;
}
The following is cheaper as we don't need to call bms_membership() and
bms_singleton_member() for singleton sets. It also saves function call
overhead for empty sets.
if (relids == NULL)
/* handle empty set */
else
{
int relid;
if (bms_get_singleton(relids, &relid))
/* handle singleton set */
else
/* handle multi-member set */
}
In the attached, I've adjusted the code to use the latter of the two
above methods in 3 places. In examine_variable() this reduces the
complexity of the logic quite a bit and saves calling bms_is_member()
in addition to bms_singleton_member().
I'm trying to reduce the footprint of what's being worked on in [1]
and I highlighted this as something that'll help with that.
Any objections to me pushing the attached?
David
[1]
https://postgr.es/m/caaphdvqhcnkji9crqzg-reqdxtfrwnt5rhzntdqhnrbzaau...@mail.gmail.com
diff --git a/src/backend/optimizer/plan/initsplan.c
b/src/backend/optimizer/plan/initsplan.c
index b31d892121..83aeca36d6 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2634,10 +2634,12 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
Relids relids = restrictinfo->required_relids;
RelOptInfo *rel;
- switch (bms_membership(relids))
+ if (!bms_is_empty(relids))
{
- case BMS_SINGLETON:
+ int relid;
+ if (bms_get_singleton_member(relids, &relid))
+ {
/*
* There is only one relation participating in the
clause, so it
* is a restriction clause for that relation.
@@ -2650,9 +2652,9 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
/* Update security level info */
rel->baserestrict_min_security =
Min(rel->baserestrict_min_security,
restrictinfo->security_level);
- break;
- case BMS_MULTIPLE:
-
+ }
+ else
+ {
/*
* The clause is a join clause, since there is more
than one rel
* in its relid set.
@@ -2675,15 +2677,15 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
* Add clause to the join lists of all the relevant
relations.
*/
add_join_clause_to_rels(root, restrictinfo, relids);
- break;
- default:
-
- /*
- * clause references no rels, and therefore we have no
place to
- * attach it. Shouldn't get here if callers are
working properly.
- */
- elog(ERROR, "cannot cope with variable-free clause");
- break;
+ }
+ }
+ else
+ {
+ /*
+ * clause references no rels, and therefore we have no place to
attach
+ * it. Shouldn't get here if callers are working properly.
+ */
+ elog(ERROR, "cannot cope with variable-free clause");
}
}
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 35c9e3c86f..e11d022827 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5028,22 +5028,27 @@ examine_variable(PlannerInfo *root, Node *node, int
varRelid,
onerel = NULL;
- switch (bms_membership(varnos))
+ if (bms_is_empty(varnos))
{
- case BMS_EMPTY_SET:
- /* No Vars at all ... must be pseudo-constant clause */
- break;
- case BMS_SINGLETON:
- if (varRelid == 0 || bms_is_member(varRelid, varnos))
+ /* No Vars at all ... must be pseudo-constant clause */
+ }
+ else
+ {
+ int relid;
+
+ if (bms_get_singleton_member(varnos, &relid))
+ {
+ if (varRelid == 0 || varRelid == relid)
{
- onerel = find_base_rel(root,
-
(varRelid ? varRelid : bms_singleton_member(varnos)));
+ onerel = find_base_rel(root, relid);
vardata->rel = onerel;
node = basenode; /* strip any relabeling
*/
}
/* else treat it as a constant */
- break;
- case BMS_MULTIPLE:
+ }
+ else
+ {
+ /* varnos has multiple relids */
if (varRelid == 0)
{
/* treat it as a variable of a join relation */
@@ -5058,7 +5063,7 @@ examine_variable(PlannerInfo *root, Node *node, int
varRelid,
/* note: no point in expressional-index search
here */
}
/* else treat it as a constant */
- break;
+ }
}
bms_free(varnos);
@@ -6381,17 +6386,14 @@ find_join_input_rel(PlannerInfo *root, Relids relids)
{
RelOptInfo *rel = NULL;
- switch (bms_membership(relids))
+ if (!bms_is_empty(relids))
{
- case BMS_EMPTY_SET:
- /* should not happen */
- break;
- case BMS_SINGLETON:
- rel = find_base_rel(root, bms_singleton_member(relids));
- break;
- case BMS_MULTIPLE:
+ int relid;
+
+ if (bms_get_singleton_member(relids, &relid))
+ rel = find_base_rel(root, relid);
+ else
rel = find_join_rel(root, relids);
- break;
}
if (rel == NULL)
