Optimize a few list_delete_ptr calls

There is a handful of places where we called list_delete_ptr() to remove
some element from a List.  In many of these places we know, or with very
little additional effort know the index of the ListCell that we need to
remove.

Here we change all of those places to instead either use one of;
list_delete_nth_cell(), foreach_delete_current() or list_delete_last().
Each of these saves from having to iterate over the list to search for the
element to remove by its pointer value.

There are some small performance gains to be had by doing this, but in the
general case, none of these lists are likely to be very large, so the
lookup was probably never that expensive anyway.  However, some of the
calls are in fairly hot code paths, e.g process_equivalence().  So any
small gains there are useful.

Author: Zhijie Hou and David Rowley
Discussion: https://postgr.es/m/b3517353ec7c4f87aa560678fbb1034b@G08CNEXMBPEKD05.g08.fujitsu.local

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 823422edad02ff3db74d1c11c49df20220a7a606..690b753369e8c2ecc2912b3a60e74eb78b399067 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -137,6 +137,7 @@ process_equivalence(PlannerInfo *root,
    EquivalenceMember *em1,
               *em2;
    ListCell   *lc1;
+   int         ec2_idx;
 
    /* Should not already be marked as having generated an eclass */
    Assert(restrictinfo->left_ec == NULL);
@@ -258,6 +259,7 @@ process_equivalence(PlannerInfo *root,
     */
    ec1 = ec2 = NULL;
    em1 = em2 = NULL;
+   ec2_idx = -1;
    foreach(lc1, root->eq_classes)
    {
        EquivalenceClass *cur_ec = (EquivalenceClass *) lfirst(lc1);
@@ -311,6 +313,7 @@ process_equivalence(PlannerInfo *root,
                equal(item2, cur_em->em_expr))
            {
                ec2 = cur_ec;
+               ec2_idx = foreach_current_index(lc1);
                em2 = cur_em;
                if (ec1)
                    break;
@@ -371,7 +374,7 @@ process_equivalence(PlannerInfo *root,
        ec1->ec_max_security = Max(ec1->ec_max_security,
                                   ec2->ec_max_security);
        ec2->ec_merged = ec1;
-       root->eq_classes = list_delete_ptr(root->eq_classes, ec2);
+       root->eq_classes = list_delete_nth_cell(root->eq_classes, ec2_idx);
        /* just to avoid debugging confusion w/ dangling pointers: */
        ec2->ec_members = NIL;
        ec2->ec_sources = NIL;
@@ -1964,6 +1967,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
        bool        matchleft;
        bool        matchright;
        ListCell   *lc2;
+       int         coal_idx = -1;
 
        /* Ignore EC unless it contains pseudoconstants */
        if (!cur_ec->ec_has_const)
@@ -2008,6 +2012,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
 
                if (equal(leftvar, cfirst) && equal(rightvar, csecond))
                {
+                   coal_idx = foreach_current_index(lc2);
                    match = true;
                    break;
                }
@@ -2072,7 +2077,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
         */
        if (matchleft && matchright)
        {
-           cur_ec->ec_members = list_delete_ptr(cur_ec->ec_members, coal_em);
+           cur_ec->ec_members = list_delete_nth_cell(cur_ec->ec_members, coal_idx);
            return true;
        }
 

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index db54a6ba2ea9268b721b3a62f74c8445be1e8028..4a35903b29f7447c34cd28c7251060c922bd333c 100644 (file)
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -1005,8 +1005,8 @@ sort_inner_and_outer(PlannerInfo *root,
        /* Make a pathkey list with this guy first */
        if (l != list_head(all_pathkeys))
            outerkeys = lcons(front_pathkey,
-                             list_delete_ptr(list_copy(all_pathkeys),
-                                             front_pathkey));
+                             list_delete_nth_cell(list_copy(all_pathkeys),
+                                                  foreach_current_index(l)));
        else
            outerkeys = all_pathkeys;   /* no work at first one... */
 

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index d24420c58319fcf2d7088691a6ee5650c837dc2c..f5165863d7791f5a582ea79c24bc546a85da74b7 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -1698,11 +1698,12 @@ transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref)
        /*
         * If we're at the last column, delete the RowExpr from
         * p_multiassign_exprs; we don't need it anymore, and don't want it in
-        * the finished UPDATE tlist.
+        * the finished UPDATE tlist.  We assume this is still the last entry
+        * in p_multiassign_exprs.
         */
        if (maref->colno == maref->ncolumns)
            pstate->p_multiassign_exprs =
-               list_delete_ptr(pstate->p_multiassign_exprs, tle);
+               list_delete_last(pstate->p_multiassign_exprs);
 
        return result;
    }

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 0dc03dd984081c9b88ff306b25a1187d1cb2c795..015b0538e339111c0442a12729d50b2a6ac6f7e3 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -360,6 +360,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
    CreateSeqStmt *seqstmt;
    AlterSeqStmt *altseqstmt;
    List       *attnamelist;
+   int         nameEl_idx = -1;
 
    /*
     * Determine namespace and name to use for the sequence.
@@ -386,6 +387,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
                        (errcode(ERRCODE_SYNTAX_ERROR),
                         errmsg("conflicting or redundant options")));
            nameEl = defel;
+           nameEl_idx = foreach_current_index(option);
        }
    }
 
@@ -405,7 +407,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
        }
        sname = rv->relname;
        /* Remove the SEQUENCE NAME item from seqoptions */
-       seqoptions = list_delete_ptr(seqoptions, nameEl);
+       seqoptions = list_delete_nth_cell(seqoptions, nameEl_idx);
    }
    else
    {

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index fe777c3103dfe557829a248cb23e88ee64dd528a..1faaafab08a6a25509f2d0e11862423e9c642f23 100644 (file)
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -650,11 +650,7 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
            if (IsA(rtr, RangeTblRef) &&
                rtr->rtindex == rt_index)
            {
-               newjointree = list_delete_ptr(newjointree, rtr);
-
-               /*
-                * foreach is safe because we exit loop after list_delete...
-                */
+               newjointree = foreach_delete_current(newjointree, l);
                break;
            }
        }