Remove explicit FreeExprContext calls during plan node shutdown. The

ExprContexts will be freed anyway when FreeExecutorState() is reached,
and letting that routine do the work is more efficient because it will
automatically free the ExprContexts in reverse creation order.  The
existing coding was effectively freeing them in exactly the worst
possible order, resulting in O(N^2) behavior inside list_delete_ptr,
which becomes highly visible in cases with a few thousand plan nodes.

ExecFreeExprContext is now effectively a no-op and could be removed,
but I left it in place in case we ever want to put it back to use.

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 040e9dcaef08691b0d8c8c69be60784b30c3dcc5..905c7f89f61ad14cc313ad7e0d8da36945d8dc71 100644 (file)
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.121 2005/04/14 20:03:24 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.122 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -620,27 +620,26 @@ ExecAssignProjectionInfo(PlanState *planstate)
 /* ----------------
  *             ExecFreeExprContext
  *
- * A plan node's ExprContext should be freed explicitly during ExecEndNode
- * because there may be shutdown callbacks to call.  (Other resources made
- * by the above routines, such as projection info, don't need to be freed
+ * A plan node's ExprContext should be freed explicitly during executor
+ * shutdown because there may be shutdown callbacks to call.  (Other resources
+ * made by the above routines, such as projection info, don't need to be freed
  * explicitly because they're just memory in the per-query memory context.)
+ *
+ * However ... there is no particular need to do it during ExecEndNode,
+ * because FreeExecutorState will free any remaining ExprContexts within
+ * the EState.  Letting FreeExecutorState do it allows the ExprContexts to
+ * be freed in reverse order of creation, rather than order of creation as
+ * will happen if we delete them here, which saves O(N^2) work in the list
+ * cleanup inside FreeExprContext.
  * ----------------
  */
 void
 ExecFreeExprContext(PlanState *planstate)
 {
-       ExprContext *econtext;
-
        /*
-        * get expression context.      if NULL then this node has none so we just
-        * return.
+        * Per above discussion, don't actually delete the ExprContext.
+        * We do unlink it from the plan node, though.
         */
-       econtext = planstate->ps_ExprContext;
-       if (econtext == NULL)
-               return;
-
-       FreeExprContext(econtext);
-
        planstate->ps_ExprContext = NULL;
 }
 

diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
index c877d69b9be133ab1c7dffce6079131ceb055f24..fafe406e1731fe159110bccf12d531f2d570b0b1 100644 (file)
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.3 2005/04/22 21:58:31 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.4 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -220,11 +220,13 @@ ExecEndBitmapIndexScan(BitmapIndexScanState *node)
        relation = node->ss.ss_currentRelation;
 
        /*
-        * Free the exprcontext(s)
+        * Free the exprcontext(s) ... now dead code, see ExecFreeExprContext
         */
+#ifdef NOT_USED
        ExecFreeExprContext(&node->ss.ps);
        if (node->biss_RuntimeContext)
                FreeExprContext(node->biss_RuntimeContext);
+#endif
 
        /*
         * close the index relation

diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 6546f4797082450a0f8340088e77cc64dd899d29..27cc29a62b4ebfbed541b4e6a7147447bb6d6c26 100644 (file)
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.100 2005/03/16 21:38:07 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.101 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -534,11 +534,13 @@ ExecEndIndexScan(IndexScanState *node)
        relation = node->ss.ss_currentRelation;
 
        /*
-        * Free the exprcontext(s)
+        * Free the exprcontext(s) ... now dead code, see ExecFreeExprContext
         */
+#ifdef NOT_USED
        ExecFreeExprContext(&node->ss.ps);
        if (node->iss_RuntimeContext)
                FreeExprContext(node->iss_RuntimeContext);
+#endif
 
        /*
         * clear out tuple table slots