From efdcca55a3df27a12efb84a18bce6ea739927b80 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 22 Jul 2019 19:03:12 +1200 Subject: [PATCH] Make better use of the new List implementation in a couple of places In nodeAppend.c and nodeMergeAppend.c there were some foreach loops which looped over the list of subplans and only performed any work if the subplan index was found in a Bitmapset. With the old linked list implementation of List, this form made sense as accessing the Nth list element was O(N). However, thanks to 1cff1b95a we now have array-based lists, so accessing the Nth element has become O(1). Here we make the most of the O(1) lookups and just loop over the set members of the Bitmapset with bms_next_member(). This performs slightly better when a small number of the list items are in the Bitmapset. Micro benchmarks show that when the Bitmapset contains all or most of the list items then the new code is ever so slightly slower. In practice, the cost is so small that it's drowned out by various other things such as locking the relations belonging to each subplan, etc. The primary goal here is to leave better code examples around which benefit better from the new list implementation. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAKJS1f8ZcsLVgkF4wOfRyMYTcPgLFiUAOedFC+U2vK_aFZk-BA@mail.gmail.com --- src/backend/executor/nodeAppend.c | 25 ++++++++++--------------- src/backend/executor/nodeMergeAppend.c | 14 +++++--------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index f3be2429db..5ff986ac7d 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -107,7 +107,6 @@ ExecInitAppend(Append *node, EState *estate, int eflags) int firstvalid; int i, j; - ListCell *lc; /* check for unsupported flags */ Assert(!(eflags & EXEC_FLAG_MARK)); @@ -211,24 +210,20 @@ ExecInitAppend(Append *node, EState *estate, int eflags) * * While at it, find out the first valid partial plan. */ - j = i = 0; + j = 0; firstvalid = nplans; - foreach(lc, node->appendplans) + i = -1; + while ((i = bms_next_member(validsubplans, i)) >= 0) { - if (bms_is_member(i, validsubplans)) - { - Plan *initNode = (Plan *) lfirst(lc); + Plan *initNode = (Plan *) list_nth(node->appendplans, i); - /* - * Record the lowest appendplans index which is a valid partial - * plan. - */ - if (i >= node->first_partial_plan && j < firstvalid) - firstvalid = j; + /* + * Record the lowest appendplans index which is a valid partial plan. + */ + if (i >= node->first_partial_plan && j < firstvalid) + firstvalid = j; - appendplanstates[j++] = ExecInitNode(initNode, estate, eflags); - } - i++; + appendplanstates[j++] = ExecInitNode(initNode, estate, eflags); } appendstate->as_first_partial_plan = firstvalid; diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c index 7ba53ba185..18d13377dc 100644 --- a/src/backend/executor/nodeMergeAppend.c +++ b/src/backend/executor/nodeMergeAppend.c @@ -70,7 +70,6 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) int nplans; int i, j; - ListCell *lc; /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); @@ -177,16 +176,13 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) * call ExecInitNode on each of the valid plans to be executed and save * the results into the mergeplanstates array. */ - j = i = 0; - foreach(lc, node->mergeplans) + j = 0; + i = -1; + while ((i = bms_next_member(validsubplans, i)) >= 0) { - if (bms_is_member(i, validsubplans)) - { - Plan *initNode = (Plan *) lfirst(lc); + Plan *initNode = (Plan *) list_nth(node->mergeplans, i); - mergeplanstates[j++] = ExecInitNode(initNode, estate, eflags); - } - i++; + mergeplanstates[j++] = ExecInitNode(initNode, estate, eflags); } mergestate->ps.ps_ProjInfo = NULL; -- 2.40.0