From e7c2b95d37a2b9c01367f7ccc55703555b39c81c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 22 Oct 2020 14:36:32 +1300 Subject: [PATCH] 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 --- src/backend/optimizer/path/equivclass.c | 9 +++++++-- src/backend/optimizer/path/joinpath.c | 4 ++-- src/backend/parser/parse_expr.c | 5 +++-- src/backend/parser/parse_utilcmd.c | 4 +++- src/backend/rewrite/rewriteHandler.c | 6 +----- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 823422edad0..690b753369e 100644 --- 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 db54a6ba2ea..4a35903b29f 100644 --- 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 d24420c5831..f5165863d77 100644 --- 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 0dc03dd9840..015b0538e33 100644 --- 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 fe777c3103d..1faaafab08a 100644 --- 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; } }